diff --git a/apps/docs/content/docs/dev/content-engine/limitations.mdx b/apps/docs/content/docs/dev/content-engine/limitations.mdx index 2f64c5295..c9759c68b 100644 --- a/apps/docs/content/docs/dev/content-engine/limitations.mdx +++ b/apps/docs/content/docs/dev/content-engine/limitations.mdx @@ -26,7 +26,7 @@ other 20%, so you find out here rather than halfway through building. | Deep relation nesting in a public response | One level, one key. Anything more is a custom route | | Origin `Cache-Control` headers | [Tags](/docs/dev/content-engine/caching) handle Next-side caching; add headers in your own route | | Per-route rate limits | The global IP bucket applies; anything finer is yours | -| Search indexing | Register a `SearchIndexer` yourself | +| Search: author facet, per-locale documents, relation containers | [`search`](/docs/dev/content-engine/search) indexes public text fields. Anything richer needs your own `SearchIndexer` | | Automatic field renames | See below | | Unique constraints on more than one column | Declare them in `indexes`, not on the field | | Uniqueness on non-`text` fields | Declare the index in `indexes` | @@ -132,16 +132,60 @@ suffixes in the [slug backfill recipe](/docs/dev/content-engine/slug-field#migrations) are a one-off, deterministic migration device for rows that predate the column. -## Direct service calls invalidate no cache +## Direct service calls invalidate no cache and index nothing `service.publish()` and friends change rows and return the result. They emit no -event and expire no cache tag: they may be inside an uncommitted transaction, -they may be running in `apps/api` where there is no Next runtime at all, and the -Next cache APIs need a request scope a repository does not own. +event, expire no cache tag and touch no search document: they may be inside an +uncommitted transaction, they may be running in `apps/api` where there is no Next +runtime at all, and the Next cache APIs need a request scope a repository does +not own. Every generated write path goes through an AdminCP server action, which does all three. A direct caller does its own follow-up, after committing - see -[Caching](/docs/dev/content-engine/caching#writing-who-expires-what). +[Caching](/docs/dev/content-engine/caching#writing-who-expires-what) and +[Search](/docs/dev/content-engine/search#direct-service-calls-do-not-synchronize). + +## A search engine outage does not undo a write + +Search synchronization is best effort. A content mutation succeeds, the failure is +logged with a `[content-search]` prefix and surfaced in the AdminCP, and a rebuild +repairs the drift. There is no automatic retry and no durable outbox, so the index +is eventually consistent - bounded by the next publish or the next rebuild. + +Logging is best effort in the same way: it writes to the database, so it can fail +alongside the search engine. When it does, the message falls back to the console +and the mutation still succeeds. Neither failure can turn a committed write into a +failed request. + +## A collection with no rebuild indexer cannot be rebuilt + +A collection with indexed documents but no registered `SearchIndexer` is labelled +**Unmanaged**. That happens when a content type drops its `search` block or its +plugin goes away - and also when a plugin simply indexes through `search.index()` +and never registers an indexer, which is supported and may be perfectly healthy. +The label therefore says only what is known: there is no way to rebuild it. + +The AdminCP names the owner stored on those documents rather than reassigning them +to core, and falls back to `unknown` only when neither a registered indexer nor the +stored rows know. It reports no coverage, because there is no source to compare +against. + +A scoped rebuild of it is refused - it would clear the documents and refill nothing +- so the only action offered is an explicit, confirmed **Remove documents**. That +clears the current indexed state and nothing more: a live-writing plugin may +recreate those documents immediately. A full rebuild clears them too, as part of +clearing everything, and recreates only collections that have an indexer. + +Registering an indexer for the same `itemType` makes the collection ordinary again. +Nothing recovers it automatically. + +## Malformed published data reads as under-indexed + +A published record the search mapper cannot project - a title that is blank in the +database, say - counts toward a collection's source total and produces no document. +The AdminCP shows that as **stale** rather than hiding it. Extra documents, for +records that no longer qualify, are stale too: the counts have to match exactly for +a collection to read as indexed. ## The public cursor is always the row id diff --git a/apps/docs/content/docs/dev/content-engine/meta.json b/apps/docs/content/docs/dev/content-engine/meta.json index 53139393c..844221bf8 100644 --- a/apps/docs/content/docs/dev/content-engine/meta.json +++ b/apps/docs/content/docs/dev/content-engine/meta.json @@ -13,6 +13,7 @@ "slug-field", "public-api", "public-service", + "search", "caching", "admincp", "permissions", diff --git a/apps/docs/content/docs/dev/content-engine/public-api.mdx b/apps/docs/content/docs/dev/content-engine/public-api.mdx index bdcb7f3ec..55bc87477 100644 --- a/apps/docs/content/docs/dev/content-engine/public-api.mdx +++ b/apps/docs/content/docs/dev/content-engine/public-api.mdx @@ -274,6 +274,13 @@ response schema generated from the projection. Only `get` operations are ever built - there is no public create, update, delete, publish or unpublish, and no flag that would add one. +## The allowlist bounds search too + +[`search`](/docs/dev/content-engine/search) can only index fields that are in +`fields`, and that is a compile error rather than a convention - a private value +must not surface through a result snippet, a highlighted match or an exact-match +probe either. + ## What this is not No public writes, ever. No `Cache-Control` headers - HTTP caching is handled diff --git a/apps/docs/content/docs/dev/content-engine/publication.mdx b/apps/docs/content/docs/dev/content-engine/publication.mdx index 6050abf2f..99f4294cc 100644 --- a/apps/docs/content/docs/dev/content-engine/publication.mdx +++ b/apps/docs/content/docs/dev/content-engine/publication.mdx @@ -271,6 +271,17 @@ path - which is exactly what the [public field allowlist](/docs/dev/content-engine/public-api#the-field-allowlist) does for you on the generated one. +## Publishing can do more than flip a column + +Two things hang off the lifecycle, and both are opt-in: + +- [`publicApi`](/docs/dev/content-engine/public-api) makes a published record + readable over HTTP. +- [`search`](/docs/dev/content-engine/search) keeps a published record in the + site-wide search index - added on publish, removed on unpublish. + +Enabling publication on its own still publishes nothing anywhere. + ## What this is not Scheduled publishing, approval workflows and revisions are not here. The diff --git a/apps/docs/content/docs/dev/content-engine/search.mdx b/apps/docs/content/docs/dev/content-engine/search.mdx new file mode 100644 index 000000000..d2f018e8a --- /dev/null +++ b/apps/docs/content/docs/dev/content-engine/search.mdx @@ -0,0 +1,366 @@ +--- +title: Search +description: One config block and every published record stays in the site-wide search index - added on publish, rewritten on edit, removed on unpublish. +icon: Search +--- + +Add `search` to a content type and VitNode keeps its published records in the +[Search Engine](/docs/dev/search) for you. No indexer to write, no reindex call +to remember in five different routes. + +```ts title="src/content/article.ts" +export const articleContentType = defineContentType({ + id: "example.article", + tableName: "example_articles", + + fields: { + title: field.text({ required: true, maxLength: 200 }), + slug: field.slug({ source: "title" }), + excerpt: field.textarea({ maxLength: 500, nullable: true }), + code: field.text({ required: true }), + author: field.user(), + }, + + publication: { enabled: true }, + + publicApi: { + enabled: true, + path: "articles", + fields: ["title", "slug", "excerpt", "publishedAt"], + }, + + search: { + enabled: true, + titleField: "title", + descriptionField: "excerpt", + contentFields: ["title", "excerpt"], + pathTemplate: "/articles/{slug}", + }, + + admin: {/* ... */}, +}); +``` + +That is the whole feature. Publish an article and it is findable at `/search`, +linking to `/articles/its-slug`. + +## The options + +| Option | Required | What it is | +| ------------------ | -------- | ----------------------------------------------------------------------------------------- | +| `enabled` | yes | Literal `true`. Omit the block, or pass `{ enabled: false }`, and nothing is indexed | +| `titleField` | yes | The result heading. A **non-nullable** `text` field, weighted above the body by the index | +| `contentFields` | yes | Concatenated into the searchable body, in order. At least one | +| `pathTemplate` | yes | The public URL of one record. Relative, with `{slug}` as the only placeholder | +| `descriptionField` | no | Leads the body, so it shows up first in a result excerpt | + +`search` is **opt-in**. Every existing content type keeps working untouched, and +so does every hand-written `SearchIndexer`. + +## Supported field kinds + +| Kind | `titleField` | `descriptionField` | `contentFields` | +| --------------------------------------- | :----------: | :----------------: | :-------------: | +| `text` | ✅ | ✅ | ✅ | +| `textarea` | ❌ | ✅ | ✅ | +| `slug` | ❌ | ❌ | ✅ | +| `enum`, `number`, `boolean`, `dateTime` | ❌ | ❌ | ❌ | +| `relation` | ❌ | ❌ | ❌ | +| `user` | ❌ | ❌ | ❌ | + +A title is one line, so prose from a `textarea` in that slot would drag down +ranking for every other document in the index. It also cannot be nullable: a +`null` heading is not a search result, and a record without one is skipped - which +shows up as a collection that never reaches full coverage. The facet kinds are not text: +full-text-indexing `"draft"` or `"42"` is noise, and it would let somebody probe +values through the search box. A `relation` is a foreign key, and a `user` field +is [never public](#every-indexed-field-must-be-public). + +## Search needs publication and a public API + +```ts +publication: { + enabled: true; +} // only published records are indexed +publicApi: { + enabled: true; +} // a hit links to a public URL +``` + +Both are checked when the definition is built, so a missing one is a startup +error naming the content type - not a surprise at runtime. + +### Every indexed field must be public + +Every field you name in `search` has to be in `publicApi.fields`. This is a +**compile error**, not a review checklist: + +```ts +publicApi: { enabled: true, path: "articles", fields: ["title", "slug"] }, +search: { + enabled: true, + titleField: "title", + contentFields: ["title", "code"], // ❌ `code` is not in publicApi.fields + pathTemplate: "/articles/{slug}", +}, +``` + + + A search index leaks in more ways than a response body does. A private value + in an indexed document can surface through a result snippet, a highlighted + match, ranking, an exact-match probe - and through the plain fact that a + record *matched* something the searcher typed. Keeping the indexed set inside + the published set means none of those are possible. + + +A `user` field can never be in `publicApi.fields`, so it can never be indexed +either. The search document carries **no author** as a result: content records do +not appear in the author facet or the AdminCP user timeline. That is deliberate - +see [Limitations](#limitations). + +## URLs + +`pathTemplate` is a plain string with one placeholder: + +```ts +pathTemplate: "/articles/{slug}"; // -> /articles/getting-started +``` + +- **Relative only.** Search results are rendered into links client-side. +- **`{slug}` only.** It resolves to the one slug field `publicApi.fields` + exposes. Any other placeholder is a startup error, so a typo cannot end up in a + URL. +- **Percent-encoded** on the way in, which matters only for a slug written + straight into the database - `slugify` never produces anything that needs it. +- **No locale prefix** in this version. + +A slug change rewrites the URL in place. There is no stale document to clean up, +because a document is identified by its content type and row id, not by its URL. + +## What gets synchronized + +| You do this | Search does this | +| ----------------------------------------- | -------------------------- | +| Create a draft | nothing | +| Update a draft | nothing | +| **Publish** | add or update the document | +| Publish something already published | nothing | +| Update a published record's indexed field | update the document | +| Change a published slug | update the document's URL | +| Update a field that is not indexed | nothing | +| **Unpublish** | remove the document | +| Unpublish something already a draft | nothing | +| Delete a published record | remove the document | +| Delete a record that was published before | remove the document | +| Delete a draft that was never published | nothing | + +"Published" is the same rule the public API uses: +`status = 'published' AND publishedAt IS NOT NULL AND publishedAt <= now()`. +There is no second definition to drift from it, so a record with a cleared or +future `publishedAt` is not indexed for exactly the reason it is not public. + +A no-op writes nothing, on purpose. There is no outbox, so a listener firing on +every button press would be doing duplicate work for free. + +## Direct service calls do not synchronize + +The generated admin routes call the synchronizer for you. A direct service call +does **not** - the same rule direct calls follow for +[cache invalidation](/docs/dev/content-engine/caching#writing-who-expires-what), +and for the same reason: your call may be inside a transaction that has not +committed, and a search document pointing at a row that got rolled back is worse +than a missing one. + +Opt in explicitly, after the write has returned: + +```ts +import { syncContentSearch } from "@vitnode/core/content/server"; + +const result = await model.service(c).publish(id); +if (result) { + await syncContentSearch(c, articleContentType, { + operation: "publish", + changed: result.changed, + row: result.row, + }); +} +``` + +Inside a transaction, put the call **after** the commit: + +```ts +const result = await db.transaction(async tx => + model.service(c).publish(id, { tx }), +); +if (result) + await syncContentSearch(c, definition, { operation: "publish", ...result }); +``` + +It needs a Hono `Context` (for `c.get("search")`), so it runs in the API process. +A Next.js server action cannot reach the search engine at all. + +## When search fails + +A search engine outage never turns a successful write into a failed one. + +``` +the record is saved -> the response is a normal 200 / 201 +the document is not -> logged, and repaired by a rebuild +``` + +The failure is written to `core_logs` behind a `[content-search]` prefix with the +content type, the owning plugin, the record id, the operation and the error, and +the newest few show up in **AdminCP → Advanced → Search** as _Recent sync +failures_. There is no automatic retry. + +Logging is best effort too. It writes to the database, so it can be down for the +same reason the search engine is - and a failed log entry must not fail the +request either. When the structured entry cannot be persisted, the message goes to +the console instead, and the returned outcome still carries the **original search +error** rather than the logger's. + + + With the bundled Postgres engine the window is tiny: the document is written + to the same database as the record, so it fails essentially only when that + database is down - in which case the record did not save either. With an + external engine like [Elasticsearch](/docs/dev/search) the index can genuinely + drift, and a rebuild is what closes the gap. + + +## Rebuild + +**AdminCP → Advanced → Search** lists every collection with its coverage, and +rebuilds one or all of them. A content type with `search` appears there +automatically, labelled with its `admin.label.plural`. + +A rebuild reads only published records, in pages, projecting only the columns the +document needs - a private column is not even fetched. It is scoped to one +collection at a time, so rebuilding one content type can never touch another +plugin's documents, and rerunning it is harmless. + +### Source rows and documents are different counts + +A page of a rebuild reports both, and they are not interchangeable: + +- one record can produce **several** documents (one per locale, in a plugin that + indexes translations), and +- one record can produce **none** - a published row whose title is blank cannot + be a search result. + +So the rebuild advances its cursor by **source rows read**, and it stops only when +a page reads no rows at all. An empty document list does not end it: a page of +records the mapper refused would otherwise stop the rebuild before the valid +records behind it. + +Generated indexers always report both counts. A hand-written indexer may still +return a bare `SearchDocument[]` - that is [deprecated but +supported](/docs/dev/search#the-older-array-result), and it is the one case where +a filtered-empty page does end the rebuild early, because an array cannot say how +many rows produced it. + +### Coverage, and what "stale" means + +Coverage compares **published** records against indexed ones, so a mostly-draft +collection still reads 100%. Both numbers are reported as measured, and a +collection is _Indexed_ only when they match exactly: + +| Counts | Status | +| --------- | ------------------------------ | +| `0 / 0` | Not indexed | +| `0 / 10` | Not indexed | +| `5 / 10` | Stale - documents missing | +| `10 / 10` | Indexed | +| `11 / 10` | Stale - documents left over | +| `11 / —` | Unmanaged - no rebuild indexer | + +Over-indexing is a real state: documents that survive for records which no longer +qualify. The source count is never raised to hide it, and the progress bar is +clamped so it cannot draw past its track while the numbers stay truthful. + +A published record the mapper cannot project counts toward the source total but +produces no document, so **malformed data reads as under-indexed**. That is +deliberate: the alternative is reproducing every mapper rule in SQL, where the two +would eventually drift apart and hide the problem instead of showing it. + + + A rebuild is a queue task, drained by the cron job. Without a cron adapter it + never runs - the AdminCP warns you when one is missing. A large collection is + also worth an index on `(status, publishedAt)`; declare it in `indexes`. + + +## Plugin ownership + +Every search document records the plugin that owns it, and the value is the same +whichever path wrote it: + +```text +publish through the AdminCP -> pluginId = @vitnode/example +full rebuild -> pluginId = @vitnode/example +one-collection rebuild -> pluginId = @vitnode/example +``` + +That is worth stating because a rebuild runs inside the **core** cron request, so +the request's plugin is not the owner. Generated indexers carry their plugin id +and stamp it on every document; the search model uses an explicit owner in +preference to the request, and falls back to the request only when a document +names none. A generated Content Engine document is never owned by `core`. + +A hand-written indexer that names no owner is stamped with the plugin that +registered it, so it does not need changing. A blank owner counts as naming +nobody. + +A collection with documents but no registered rebuild indexer shows the owner +stored on its documents. - shows the owner stored on its +documents, and `unknown` only when neither source knows. It is never relabelled as +core. + +Such a collection is **unmanaged by the rebuild system**: it is labelled +_Unmanaged_, its coverage is left blank because there is no source to measure +against, and _Reindex_ is replaced by _Remove documents_. A scoped rebuild of it is +refused rather than silently clearing it - see [unmanaged +collections](/docs/dev/search#unmanaged-collections). + +Removing a content type's `search` block, or the plugin itself, is one way to +produce one. It is not the only way, and the label does not claim it was: a plugin +that indexes through `search.index()` without registering an indexer looks +identical, and may be entirely healthy. A full rebuild clears these documents +either way, because it recreates only collections that have an indexer. + +## Migrating a hand-written indexer + +Existing `SearchIndexer` registrations are untouched and keep working. To replace +one with the generated adapter: + +1. Add `search` to the content type, and make sure every field you index is in + `publicApi.fields`. +2. Delete your `reindex*` calls from the mutation routes, and the indexer from + `searchIndexers` in `config.api.ts`. +3. Rebuild the index once. The item type changes from your own string to the + content type id, so old documents are replaced rather than updated. + +The generated adapter does not do everything a hand-written one can. One document +per locale (the way the blog plugin indexes translations from +`core_languages_words`), an author facet, or a container relation still need your +own indexer - and one item type may only have one indexer, so it is one or the +other. + +## Limitations + +| Not supported | Why | +| ----------------------------------------------- | ---------------------------------------------------------------------------------------------------- | +| Author facet | A `user` field is never public, and the public search route resolves `authorId` into a person | +| One document per locale | Content fields are single-language columns. A document is language-agnostic and matches every locale | +| Per-locale stemming | Same reason - a language-agnostic document uses the `simple` text-search configuration | +| Relation expansion | A relation is a foreign key; the index has no place to put one | +| Locale-prefixed URLs | `pathTemplate` produces one relative path | +| A custom icon or label in the public feed | Content hits use the generic renderer. The registry in core is not plugin-extensible yet | +| Automatic retry | Best effort plus a rebuild. There is no durable retry mechanism - an outbox is a later addition | +| Keyset paging during a rebuild | The indexer contract pages by offset | +| Blank titles written straight into the database | Rejected by the mapper, and visible as an under-indexed collection | + +## Related + +- [Search Engine](/docs/dev/search) - the engine itself, and swapping it +- [Publication](/docs/dev/content-engine/publication) - the lifecycle search follows +- [Public API](/docs/dev/content-engine/public-api) - the allowlist search is bound by +- [Caching](/docs/dev/content-engine/caching) - the other thing a write has to expire diff --git a/apps/docs/content/docs/dev/search.mdx b/apps/docs/content/docs/dev/search.mdx index 9012af24a..fdaef7fe3 100644 --- a/apps/docs/content/docs/dev/search.mdx +++ b/apps/docs/content/docs/dev/search.mdx @@ -28,7 +28,16 @@ config change followed by a rebuild. ## Indexing content -Any API handler can (re)index or remove an item through `c.get("search")`: + + A content type only needs a + [`search` block](/docs/dev/content-engine/search) - publishing, editing, + unpublishing and deleting a record then keep its document in step + automatically, and it joins the rebuild without any of the wiring below. + + +Any API handler can (re)index or remove an item through `c.get("search")`. It is +an **API-process** model: a Next.js server action or server component cannot +reach `c.get("search")` at all, so anything Next-side goes over HTTP. ```ts // After creating or updating an item @@ -83,9 +92,11 @@ translated can leave `languageCode` empty (`""`) - those rows are **language agnostic** and match every locale, so single-language plugins need no changes. - Postgres full-text ranking uses the `english` text-search configuration for - all languages (there is no bundled dictionary for most locales). Matching - still works across languages; only stemming and stop-words are English-tuned. + Postgres full-text ranking picks a text-search configuration per locale + (`polish` for `pl`, `german` for `de`, and so on), falling back to `simple` for + a locale with no bundled dictionary - and for a document with no + `languageCode`, which matches every locale. Matching works across languages + either way; only stemming and stop-words differ. ## Registering a rebuild indexer @@ -106,9 +117,13 @@ export const blogPostSearchIndexer: SearchIndexer = { .limit(limit) .offset(offset); - // An indexer may emit several documents per item (e.g. one per language). - // `offset`/`limit` page over items, not documents. - return rows.flatMap(buildSearchDocumentsForPost); + // Two counts, and they are not interchangeable: one item can emit several + // documents (one per language) or none at all. `itemsRead` is what the + // rebuild pages by, and `itemsRead: 0` is what ends it. + return { + documents: rows.flatMap(buildSearchDocumentsForPost), + itemsRead: rows.length, + }; }, }; @@ -120,9 +135,119 @@ export const blogApiPlugin = () => }); ``` -Trigger a rebuild from **AdminCP → System → Search → Rebuild index**. It runs as a -background queue task, so a [cron adapter](/docs/dev/cron) must be configured for -the queue to drain. +One item type may only have **one** indexer. Two plugins claiming the same +`itemType` is a startup error naming both, because they would otherwise overwrite +each other's documents on every rebuild. + +`itemsRead` is the number of **source items** the page read, not the number of +search documents it produced. They differ whenever one item emits several +documents (one per language) or none at all (a row that cannot be projected). + + + Returning `documents.length` as `itemsRead`, or ending the loop on an empty + `documents` array, silently truncates the index: a page whose rows all fail to + project would stop the rebuild before the valid rows behind it. Report the rows + you read. + + +### The older array result + +`load` may still return a bare `SearchDocument[]`: + +```ts +load: async (c, offset, limit) => await buildDocuments(c, offset, limit), +``` + +It keeps working, so an indexer written before the page contract needs no +changes. It is **deprecated**, though, and it is not equally safe: + +- it carries no source count, so the rebuild advances by the **requested page + size** and waits for an empty array to stop; +- it therefore cannot express "rows were read, none could be projected" - such a + page looks exactly like the end of the source, and the rows behind it are never + indexed. + +Return a `SearchIndexerPage` in new code. The array form is removed only in a +future major release. + +### Ownership + +A document may name the plugin that owns it: + +```ts +{ itemType: "blog_post", itemId: 1, pluginId: "@vitnode/blog", /* ... */ } +``` + +Set it whenever you know the owner. A rebuild runs inside the **core** cron +request, so the request's plugin is not the owner - an explicit `pluginId` is what +makes a rebuilt document identical to a live-indexed one. An indexer that names +none has its registering plugin stamped on during the rebuild, so existing +indexers keep working; the request's plugin, then `"core"`, are the remaining +fallbacks. An empty or whitespace-only value counts as naming nobody. Ownership is +resolved once, in the search model, so the canonical row and the mirrored document +can never disagree. + +**AdminCP → Advanced → Search** shows the registered indexer's plugin. When a +collection has documents but no registered indexer, it shows the owner stored on +those documents instead, and `unknown` only when neither source knows. Such a +collection is never relabelled as core. + +Trigger a rebuild from **AdminCP → Advanced → Search → Rebuild index**. It runs as +a background queue task, so a [cron adapter](/docs/dev/cron) must be configured +for the queue to drain. + + + Rebuilding drops the documents it is about to replace first - the whole index + for "rebuild everything", or one collection for a single reindex. So search + returns less while it runs. + + +The two rebuilds differ in what they are allowed to destroy: + +- **A scoped rebuild requires a registered `SearchIndexer`.** It never clears an + item type it cannot rebuild - the request is rejected with a 404, and the queue + task refuses it too, in case it was dispatched directly or queued while the + indexer was still there. +- **A full rebuild clears the whole index and recreates only collections with a + registered indexer.** Documents belonging to an item type with no indexer are + therefore removed, and come back only when their plugin writes them again. On a + site with live-only indexers, that is worth knowing before clicking it. + +## Unmanaged collections + +Indexing and rebuilding are **two independent capabilities**: + +- `c.get("search").index(document)` keeps a collection current as your plugin + writes. This is all a plugin needs to be searchable. +- A registered `SearchIndexer` additionally lets the whole collection be + reproduced from its source, which is what a rebuild does. + +Registering an indexer is optional, so a plugin may write documents live and +register none. That collection works perfectly during normal operation; it simply +cannot take part in a deterministic rebuild. + +A collection is **unmanaged by the rebuild system** when documents for its +`itemType` are in the index but no `SearchIndexer` is registered for it. + + + It does **not** prove the plugin is uninstalled or inactive. A live-only plugin + looks exactly the same from the index's point of view, and may be keeping the + collection completely up to date. All VitNode can tell is that it has no way to + rebuild it. + + +**AdminCP → Advanced → Search** labels those rows *Unmanaged*, shows the plugin +stored on their documents, and says no rebuild indexer is registered. Coverage is +left blank rather than calculated: with no indexer there is no source count, and +`11 / 11` would claim a collection nothing can rebuild is fully covered. + +*Reindex* is replaced by **Remove documents**, behind a confirmation. It deletes +what is currently indexed and rebuilds nothing - but it does not stop anything +either, so a live-writing plugin may recreate those documents on its next write. +It is a way to clear a stale indexed state, not a way to uninstall a collection. + +Registering an indexer for the item type makes the collection rebuildable, and the +row becomes an ordinary one. ## Giving a type an icon and label diff --git a/packages/elasticsearch/src/index.test.ts b/packages/elasticsearch/src/index.test.ts index ad217f6bd..6c4f426a2 100644 --- a/packages/elasticsearch/src/index.test.ts +++ b/packages/elasticsearch/src/index.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const { Client, ResponseError, + bulk, index, deleteByQuery, ping, @@ -40,6 +41,7 @@ const { return { Client, ResponseError, + bulk, index, deleteByQuery, ping, @@ -159,6 +161,48 @@ describe("ElasticsearchSearchAdapter.index", () => { }); }); +describe("ElasticsearchSearchAdapter plugin ownership", () => { + it("serializes the document's owning plugin", async () => { + await ElasticsearchSearchAdapter(config).index(c, { + ...doc, + pluginId: "@vitnode/example", + }); + + expect(index).toHaveBeenCalledWith( + expect.objectContaining({ + document: expect.objectContaining({ pluginId: "@vitnode/example" }), + }), + ); + }); + + it("serializes each owner in a bulk write", async () => { + await ElasticsearchSearchAdapter(config).bulkIndex(c, [ + { ...doc, itemId: 1, pluginId: "@vitnode/example" }, + { ...doc, itemId: 2, pluginId: "@vitnode/blog" }, + ]); + + const { operations } = bulk.mock.calls[0][0] as { + operations: { pluginId?: string }[]; + }; + + // Alternating action/document pairs, so the sources are the odd entries. + expect(operations[1].pluginId).toBe("@vitnode/example"); + expect(operations[3].pluginId).toBe("@vitnode/blog"); + }); + + it("falls back to core for a document with no owner", async () => { + // `SearchModel` resolves ownership before any provider sees a document, so + // this only covers a provider called directly. + await ElasticsearchSearchAdapter(config).index(c, doc); + + expect(index).toHaveBeenCalledWith( + expect.objectContaining({ + document: expect.objectContaining({ pluginId: "core" }), + }), + ); + }); +}); + describe("ElasticsearchSearchAdapter index initialization", () => { it("creates the index only once under concurrent calls", async () => { exists.mockResolvedValue(false); diff --git a/packages/elasticsearch/src/index.ts b/packages/elasticsearch/src/index.ts index e1dddb680..8778787f8 100644 --- a/packages/elasticsearch/src/index.ts +++ b/packages/elasticsearch/src/index.ts @@ -60,7 +60,10 @@ const isIndexAlreadyExistsError = (error: unknown): boolean => "resource_already_exists_exception"; const toSource = (doc: SearchDocument): EsSource => ({ - pluginId: "core", + // `SearchModel` resolves ownership before any provider sees the document, so + // this fallback is only for a provider called directly - it must never be the + // reason a mirrored document disagrees with the canonical row. + pluginId: doc.pluginId ?? "core", itemType: doc.itemType, itemId: doc.itemId, languageCode: doc.languageCode ?? "", diff --git a/packages/vitnode/src/api/lib/module.ts b/packages/vitnode/src/api/lib/module.ts index 9c2f93ef3..f033df311 100644 --- a/packages/vitnode/src/api/lib/module.ts +++ b/packages/vitnode/src/api/lib/module.ts @@ -2,6 +2,7 @@ import { OpenAPIHono } from "@hono/zod-openapi"; import type { AnyContentTypeDefinition } from "@/content/types"; +import type { SearchIndexer } from "../models/search"; import type { BuildCronReturn } from "./cron"; import type { BuildEventListenerReturn } from "./events"; import type { BuildQueueTaskReturn } from "./queue"; @@ -33,6 +34,7 @@ export interface BaseBuildModuleReturn< pluginId: P; queueTasks: BuildQueueTaskReturn[]; routes: Routes; + searchIndexers?: SearchIndexer[]; webSockets: BuildWebSocketReturn[]; } @@ -59,6 +61,7 @@ export function buildModule< cronJobs = [], events = [], queueTasks = [], + searchIndexers, webSockets = [], }: { contentTypes?: AnyContentTypeDefinition[]; @@ -69,6 +72,7 @@ export function buildModule< pluginId: P; queueTasks?: BuildQueueTaskReturn[]; routes: Routes; + searchIndexers?: SearchIndexer[]; webSockets?: BuildWebSocketReturn[]; }): BuildModuleReturn { const hono = new OpenAPIHono(); @@ -95,6 +99,7 @@ export function buildModule< cronJobs, events, queueTasks, + searchIndexers, webSockets, }; } diff --git a/packages/vitnode/src/api/lib/plugin.test.ts b/packages/vitnode/src/api/lib/plugin.test.ts index 72a8d9eee..af7eb2e0a 100644 --- a/packages/vitnode/src/api/lib/plugin.test.ts +++ b/packages/vitnode/src/api/lib/plugin.test.ts @@ -6,6 +6,9 @@ import { testCategoryContentType, } from "@/tests/content-fixtures"; +import type { SearchIndexer } from "../models/search"; + +import { validateSearchIndexers } from "../models/search"; import { buildModule } from "./module"; import { buildApiPlugin } from "./plugin"; @@ -89,3 +92,128 @@ describe("buildApiPlugin content types", () => { ).toThrow(/Duplicate content type id/); }); }); + +const indexer = (itemType: string): SearchIndexer => ({ + itemType, + load: async () => await Promise.resolve({ documents: [], itemsRead: 0 }), +}); + +describe("buildApiPlugin search indexers", () => { + it("collects indexers from nested modules", () => { + const nested = buildModule({ + pluginId: "@vitnode/example", + name: "content", + routes: [], + searchIndexers: [indexer("test.article")], + }); + + const plugin = buildApiPlugin({ + pluginId: "@vitnode/example", + modules: [ + buildModule({ + pluginId: "@vitnode/example", + name: "admin", + routes: [], + modules: [nested], + }), + ], + }); + + expect(plugin.searchIndexers?.map(item => item.itemType)).toEqual([ + "test.article", + ]); + }); + + it("merges root-level indexers with collected ones", () => { + const plugin = buildApiPlugin({ + pluginId: "@vitnode/example", + modules: [ + buildModule({ + pluginId: "@vitnode/example", + name: "content", + routes: [], + searchIndexers: [indexer("test.article")], + }), + ], + searchIndexers: [indexer("blog_post")], + }); + + expect(plugin.searchIndexers?.map(item => item.itemType)).toEqual([ + "blog_post", + "test.article", + ]); + }); + + it("leaves a plugin with no indexers alone", () => { + const plugin = buildApiPlugin({ + pluginId: "@vitnode/example", + modules: [adminModule], + }); + + expect(plugin.searchIndexers).toEqual([]); + }); + + it("rejects the same item type registered twice", () => { + expect(() => + buildApiPlugin({ + pluginId: "@vitnode/example", + modules: [ + buildModule({ + pluginId: "@vitnode/example", + name: "content", + routes: [], + searchIndexers: [indexer("test.article")], + }), + ], + searchIndexers: [indexer("test.article")], + }), + ).toThrow(/Duplicate search indexer for item type "test.article"/); + }); +}); + +describe("validateSearchIndexers", () => { + it("retains each plugin's ownership after collection", () => { + // What the AdminCP reports a collection's owner from, and what the rebuild + // stamps on a legacy document. + const collected = [ + ...(buildApiPlugin({ + pluginId: "@vitnode/example", + searchIndexers: [indexer("test.article")], + }).searchIndexers ?? []), + ].map(item => ({ ...item, pluginId: "@vitnode/example" })); + const other = [ + ...(buildApiPlugin({ + pluginId: "@vitnode/blog", + searchIndexers: [indexer("blog_post")], + }).searchIndexers ?? []), + ].map(item => ({ ...item, pluginId: "@vitnode/blog" })); + + expect( + validateSearchIndexers([...collected, ...other]).map(item => [ + item.itemType, + item.pluginId, + ]), + ).toEqual([ + ["test.article", "@vitnode/example"], + ["blog_post", "@vitnode/blog"], + ]); + }); + + it("names both owners of a collision", () => { + expect(() => + validateSearchIndexers([ + { ...indexer("blog_post"), pluginId: "@vitnode/blog" }, + { ...indexer("blog_post"), pluginId: "@vitnode/other" }, + ]), + ).toThrow(/both "@vitnode\/blog" and "@vitnode\/other"/); + }); + + it("passes distinct item types through", () => { + expect( + validateSearchIndexers([ + { ...indexer("blog_post"), pluginId: "@vitnode/blog" }, + { ...indexer("test.article"), pluginId: "@vitnode/example" }, + ]).map(item => item.itemType), + ).toEqual(["blog_post", "test.article"]); + }); +}); diff --git a/packages/vitnode/src/api/lib/plugin.ts b/packages/vitnode/src/api/lib/plugin.ts index d3abc12d5..29306c788 100644 --- a/packages/vitnode/src/api/lib/plugin.ts +++ b/packages/vitnode/src/api/lib/plugin.ts @@ -17,6 +17,7 @@ import type { PermissionStaffConfig } from "./permission-staff"; import type { QueueTaskConfig } from "./queue"; import type { WebSocketConfig } from "./websocket"; +import { validateSearchIndexers } from "../models/search"; import { checkPluginId } from "./check-plugin-id"; export interface BuildPluginApiReturn { @@ -58,12 +59,14 @@ export function buildApiPlugin

({ const contentTypes: AnyContentTypeDefinition[] = []; const cronJobs: BuildPluginApiReturn["cronJobs"] = []; const events: BuildPluginApiReturn["events"] = []; + const indexers: SearchIndexer[] = [...(searchIndexers ?? [])]; const queueTasks: BuildPluginApiReturn["queueTasks"] = []; const webSockets: BuildPluginApiReturn["webSockets"] = []; modules.forEach(handler => { hono.route(`/${handler.name}`, handler.hono); contentTypes.push(...collectContentTypes(handler)); + indexers.push(...collectSearchIndexers(handler)); handler.cronJobs?.forEach(cron => { cronJobs.push({ ...cron, module: handler.name }); @@ -86,6 +89,8 @@ export function buildApiPlugin

({ contentTypes.map(definition => ({ definition, pluginId })), ); + validateSearchIndexers(indexers.map(indexer => ({ ...indexer, pluginId }))); + return { pluginId, messages, @@ -94,7 +99,7 @@ export function buildApiPlugin

({ cronJobs, events, queueTasks, - searchIndexers, + searchIndexers: indexers, webSockets, // Every content type contributes can_view/can_create/can_edit/can_delete // unless the plugin declared that module itself. @@ -116,3 +121,10 @@ function collectContentTypes( ...(module.modules ?? []).flatMap(collectContentTypes), ]; } + +function collectSearchIndexers(module: BaseBuildModuleReturn): SearchIndexer[] { + return [ + ...(module.searchIndexers ?? []), + ...(module.modules ?? []).flatMap(collectSearchIndexers), + ]; +} diff --git a/packages/vitnode/src/api/middlewares/global.middleware.ts b/packages/vitnode/src/api/middlewares/global.middleware.ts index 47282cff0..5c5a366a3 100644 --- a/packages/vitnode/src/api/middlewares/global.middleware.ts +++ b/packages/vitnode/src/api/middlewares/global.middleware.ts @@ -16,7 +16,7 @@ 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 } from "@/api/models/search"; +import { 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"; @@ -215,12 +215,15 @@ export const globalMiddleware = ({ })), ); - const searchIndexersMetadata: SearchIndexerConfig[] = plugins.flatMap( - plugin => + // Validated across *all* plugins, for the same reason content types are: + // `buildApiPlugin` can only catch collisions inside a single plugin. + const searchIndexersMetadata: SearchIndexerConfig[] = validateSearchIndexers( + plugins.flatMap(plugin => (plugin.searchIndexers ?? []).map(indexer => ({ ...indexer, pluginId: plugin.pluginId, })), + ), ); // Validated once more across *all* plugins: `buildApiPlugin` can only catch diff --git a/packages/vitnode/src/api/models/search.test-d.ts b/packages/vitnode/src/api/models/search.test-d.ts new file mode 100644 index 000000000..20234211b --- /dev/null +++ b/packages/vitnode/src/api/models/search.test-d.ts @@ -0,0 +1,90 @@ +/* eslint-disable @typescript-eslint/no-deprecated -- Asserting that the + deprecated result shape still compiles is the point of this file. */ +import { assertType, describe, expectTypeOf, it } from "vitest"; + +import type { ContentSearchIndexer } from "@/content/server"; + +import type { + LegacySearchIndexerPage, + SearchDocument, + SearchIndexer, + SearchIndexerLoadResult, + SearchIndexerPage, +} from "./search"; + +const document: SearchDocument = { + content: "body", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + itemId: 1, + itemType: "custom_item", + title: "Hello", +}; + +describe("SearchIndexer.load", () => { + it("accepts the preferred page result", () => { + assertType({ + itemType: "custom_item", + load: async (_c, _offset, limit) => + await Promise.resolve({ documents: [document], itemsRead: limit }), + }); + }); + + it("still accepts the deprecated array result", () => { + // Exactly what a plugin written before Stage 3 looks like. It has to keep + // compiling: `SearchIndexer` is a documented public import. + const legacyIndexer: SearchIndexer = { + itemType: "legacy.item", + load: async () => await Promise.resolve([]), + }; + + assertType(legacyIndexer); + assertType({ + itemType: "legacy.item", + load: async () => await Promise.resolve([document]), + }); + }); + + it("accepts an indexer that returns either shape", () => { + assertType({ + count: async () => await Promise.resolve(1), + itemType: "either", + load: async (_c, offset) => + await Promise.resolve( + offset === 0 ? { documents: [document], itemsRead: 1 } : [], + ), + }); + }); + + it("rejects a result that is neither shape", () => { + assertType({ + itemType: "wrong", + // @ts-expect-error - a bare document is not a page and not an array. + load: async () => await Promise.resolve(document), + }); + }); + + it("rejects a page missing its source count", () => { + assertType({ + itemType: "wrong", + // @ts-expect-error - `itemsRead` is what the rebuild pages by. + load: async () => await Promise.resolve({ documents: [document] }), + }); + }); +}); + +describe("SearchIndexerLoadResult", () => { + it("is the union of the page and the deprecated array", () => { + expectTypeOf().toExtend(); + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf(); + }); +}); + +describe("ContentSearchIndexer", () => { + it("is a SearchIndexer pinned to the page result", () => { + expectTypeOf().toExtend(); + expectTypeOf< + Awaited> + >().toEqualTypeOf(); + }); +}); diff --git a/packages/vitnode/src/api/models/search.test.ts b/packages/vitnode/src/api/models/search.test.ts index 74323a5b9..e59e4b7a0 100644 --- a/packages/vitnode/src/api/models/search.test.ts +++ b/packages/vitnode/src/api/models/search.test.ts @@ -5,9 +5,9 @@ import { describe, expect, it, vi } from "vitest"; import { core_search_index } from "@/database/search"; -import type { SearchProviderApiPlugin } from "./search"; +import type { SearchDocument, SearchProviderApiPlugin } from "./search"; -import { SearchModel } from "./search"; +import { normalizeSearchIndexerPage, SearchModel } from "./search"; const createProvider = (): SearchProviderApiPlugin => ({ name: "postgres", @@ -28,7 +28,10 @@ const createProvider = (): SearchProviderApiPlugin => ({ }), }); -const createContext = (provider: SearchProviderApiPlugin) => { +const createContext = ( + provider: SearchProviderApiPlugin, + requestPluginId?: string, +) => { const onConflictDoUpdate = vi.fn().mockResolvedValue(undefined); const values = vi.fn< (row: { content: string; isPublic: boolean; pluginId: string }) => { @@ -44,7 +47,9 @@ const createContext = (provider: SearchProviderApiPlugin) => { get: (key: string) => { if (key === "db") return db; if (key === "core") return { search: { adapter: provider } }; - if (key === "plugin") return undefined; + if (key === "plugin") { + return requestPluginId ? { id: requestPluginId } : undefined; + } return undefined; }, @@ -75,6 +80,112 @@ describe("SearchModel", () => { expect(provider.index).toHaveBeenCalledWith(c, { ...doc, content: "Hello world", + pluginId: "core", + }); + }); + + describe("plugin ownership", () => { + const doc = { + content: "body", + createdAt: new Date("2026-01-01"), + itemId: 1, + itemType: "example.article", + title: "Hello", + }; + + it("prefers an explicit document owner over the request", async () => { + // A rebuild runs inside the core cron request, so the request's plugin is + // not the owner - the document has to win, or the same record would be + // stored differently depending on which path wrote it. + const provider = createProvider(); + const { c, values } = createContext(provider, "@vitnode/core"); + + await new SearchModel(c).index({ ...doc, pluginId: "@vitnode/example" }); + + expect(values.mock.calls[0][0].pluginId).toBe("@vitnode/example"); + expect(provider.index).toHaveBeenCalledWith( + c, + expect.objectContaining({ pluginId: "@vitnode/example" }), + ); + }); + + it("falls back to the request's plugin", async () => { + const provider = createProvider(); + const { c, values } = createContext(provider, "@vitnode/example"); + + await new SearchModel(c).index(doc); + + expect(values.mock.calls[0][0].pluginId).toBe("@vitnode/example"); + expect(provider.index).toHaveBeenCalledWith( + c, + expect.objectContaining({ pluginId: "@vitnode/example" }), + ); + }); + + it("treats a blank owner as absent", async () => { + // `pluginId` is public input, so an empty or whitespace-only string is a + // missing owner - not a collection called "". + const provider = createProvider(); + const { c, values } = createContext(provider, "@vitnode/example"); + + await new SearchModel(c).index({ ...doc, pluginId: " " }); + + expect(values.mock.calls[0][0].pluginId).toBe("@vitnode/example"); + }); + + it("falls back to core for a blank owner outside a plugin request", async () => { + const provider = createProvider(); + const { c, values } = createContext(provider); + + await new SearchModel(c).index({ ...doc, pluginId: "" }); + + expect(values.mock.calls[0][0].pluginId).toBe("core"); + }); + + it("falls back to core outside a plugin request", async () => { + const provider = createProvider(); + const { c, values } = createContext(provider); + + await new SearchModel(c).index(doc); + + expect(values.mock.calls[0][0].pluginId).toBe("core"); + }); + + it("resolves every document in a bulk write", async () => { + const provider = createProvider(); + const { c, values } = createContext(provider, "@vitnode/core"); + + await new SearchModel(c).bulkIndex([ + { ...doc, itemId: 1, pluginId: "@vitnode/example" }, + { ...doc, itemId: 2, pluginId: "@vitnode/blog" }, + // No owner declared: the request's plugin stands in. + { ...doc, itemId: 3 }, + ]); + + expect(values.mock.calls.map(call => call[0].pluginId)).toEqual([ + "@vitnode/example", + "@vitnode/blog", + "@vitnode/core", + ]); + expect(provider.bulkIndex).toHaveBeenCalledWith(c, [ + expect.objectContaining({ itemId: 1, pluginId: "@vitnode/example" }), + expect.objectContaining({ itemId: 2, pluginId: "@vitnode/blog" }), + expect.objectContaining({ itemId: 3, pluginId: "@vitnode/core" }), + ]); + }); + + it("rewrites the owner of an existing row on conflict", async () => { + // Otherwise a row written before its indexer declared an owner would keep + // the first writer's guess forever, and a rebuild could not repair it. + const provider = createProvider(); + const { c, values } = createContext(provider, "@vitnode/example"); + + await new SearchModel(c).index(doc); + + const { onConflictDoUpdate } = values.mock.results[0].value; + expect(onConflictDoUpdate.mock.calls[0][0].set).toMatchObject({ + pluginId: "@vitnode/example", + }); }); }); @@ -100,3 +211,49 @@ describe("SearchModel", () => { }); }); }); + +describe("normalizeSearchIndexerPage", () => { + const document: SearchDocument = { + content: "body", + createdAt: new Date("2026-01-01"), + itemId: 1, + itemType: "legacy.item", + title: "Hello", + }; + + it("passes a modern page through untouched", () => { + const page = { documents: [document], itemsRead: 7 }; + + expect(normalizeSearchIndexerPage(page, 200)).toBe(page); + }); + + it("keeps a modern page that read rows but produced nothing", () => { + // The whole reason the object form exists: this must not read as exhausted. + expect( + normalizeSearchIndexerPage({ documents: [], itemsRead: 200 }, 200), + ).toEqual({ documents: [], itemsRead: 200 }); + }); + + it("reports the requested limit for a non-empty legacy array", () => { + // Not `documents.length`: a legacy indexer may emit several documents per + // source row, so the array length would skip rows on every page. + expect(normalizeSearchIndexerPage([document], 200)).toEqual({ + documents: [document], + itemsRead: 200, + }); + }); + + it("reports the requested limit however many documents a page holds", () => { + expect( + normalizeSearchIndexerPage([document, document, document, document], 200) + .itemsRead, + ).toBe(200); + }); + + it("treats an empty legacy array as an exhausted source", () => { + expect(normalizeSearchIndexerPage([], 200)).toEqual({ + documents: [], + itemsRead: 0, + }); + }); +}); diff --git a/packages/vitnode/src/api/models/search.ts b/packages/vitnode/src/api/models/search.ts index 7a29d88fa..2ce36f1ba 100644 --- a/packages/vitnode/src/api/models/search.ts +++ b/packages/vitnode/src/api/models/search.ts @@ -19,6 +19,11 @@ export interface SearchDocument { // language; single-language content may leave it empty. languageCode?: string; metadata?: Record; + // The plugin that owns this item. Omit it and {@link SearchModel} falls back + // to the request's plugin - which is only right while the request *is* the + // owning plugin's, so a rebuild (it runs in the core cron request) must set it + // explicitly. + pluginId?: string; title: string; updatedAt?: Date; url?: string; @@ -85,10 +90,49 @@ export interface SearchProviderCapabilities { timeDecay: boolean; } +/** + * One page of a rebuild. + * + * The two counts are separate on purpose. An indexer may emit several documents + * per item (one per language, say) or none at all (a row whose data cannot be + * projected), so a document count can never stand in for a source count - using + * it would either skip items or end the rebuild while rows remain. + */ +export interface SearchIndexerPage { + documents: SearchDocument[]; + /** Source rows this page read. `0` means the source is exhausted. */ + itemsRead: number; +} + +/** + * The pre-{@link SearchIndexerPage} result: documents with no source count. + * + * @deprecated Return a {@link SearchIndexerPage}. An array cannot say how many + * source rows produced it, so the rebuild has to assume a full page was read and + * wait for an empty one to stop - which means a page that reads rows and projects + * none of them (every row on it malformed, say) ends the rebuild early and the + * rows behind it are never indexed. Supported for now; removed in a future major + * release. + */ +export type LegacySearchIndexerPage = SearchDocument[]; + +export type SearchIndexerLoadResult = + // The one intentional use of the deprecated shape: this union is what keeps + // pre-Stage-3 indexers compiling, so the lint rule has nothing to warn about + // here. Every *other* reference should be flagged. + // eslint-disable-next-line @typescript-eslint/no-deprecated + LegacySearchIndexerPage | SearchIndexerPage; + /** * Streams every existing item of one content type so the whole index can be - * rebuilt (e.g. after switching engines). `load` returns one page at a time; - * return fewer than `limit` rows to signal the end. + * rebuilt (e.g. after switching engines). + * + * `load` is called with `offset` advanced by the previous page's `itemsRead`. + * Report `itemsRead: 0` to end the rebuild; an empty `documents` array does not, + * because a page can legitimately read rows and project none of them. + * + * Returning a bare `SearchDocument[]` still works - see + * {@link LegacySearchIndexerPage} for what it gives up. */ export interface SearchIndexer { // Total number of source items available to index for this type. Powers the @@ -100,13 +144,81 @@ export interface SearchIndexer { c: Context, offset: number, limit: number, - ) => Promise; + ) => Promise; } +/** + * A declared document owner, or `undefined` when there is not really one. + * + * `pluginId` is public input, so an empty or whitespace-only string is a missing + * owner rather than a collection named `""`. Every place that resolves ownership + * goes through this, so the fallback chains cannot drift apart. + */ +export const searchDocumentOwner = ( + pluginId: null | string | undefined, +): string | undefined => { + const trimmed = pluginId?.trim(); + + return trimmed === "" ? undefined : trimmed; +}; + +/** + * Turns either `load` result into a page, so the rebuild has one shape to reason + * about and the compatibility rule lives in exactly one place. + * + * A non-empty legacy array reports `requestedLimit` rather than + * `documents.length`, because that is what the old rebuild advanced by: an + * indexer may emit several documents per source row (one per language), so a + * document count would skip rows on every page. An empty array is the only end + * signal it has. + */ +export const normalizeSearchIndexerPage = ( + result: SearchIndexerLoadResult, + requestedLimit: number, +): SearchIndexerPage => { + if (!Array.isArray(result)) return result; + + return { + documents: result, + itemsRead: result.length === 0 ? 0 : requestedLimit, + }; +}; + export interface SearchIndexerConfig extends SearchIndexer { pluginId: string; } +/** + * Rejects two indexers claiming the same `itemType`. + * + * `itemType` is the index's only namespace, so a collision is not a cosmetic + * problem: both indexers would `load` on every rebuild, writing over each + * other's documents whenever their item ids overlap, and the admin coverage + * report would silently describe only the first one. Failing at boot is the only + * place this is cheap to notice. + * + * Called once per plugin by `buildApiPlugin` and again across every plugin by + * the global middleware, which is the only place that sees them all. + */ +export const validateSearchIndexers = ( + indexers: readonly SearchIndexerConfig[], +): SearchIndexerConfig[] => { + const seen = new Map(); + + for (const indexer of indexers) { + const owner = seen.get(indexer.itemType); + if (owner !== undefined) { + throw new Error( + `[Search] Duplicate search indexer for item type "${indexer.itemType}": registered by both "${owner}" and "${indexer.pluginId}". An item type may only be indexed by one indexer.`, + ); + } + + seen.set(indexer.itemType, indexer.pluginId); + } + + return [...indexers]; +}; + /** * A pluggable search engine. The {@link SearchModel} owns the canonical * `core_search_index` table for every provider, so a provider that queries that @@ -126,7 +238,7 @@ export interface SearchProviderApiPlugin { } const toRow = (doc: SearchDocument) => ({ - pluginId: "core", + pluginId: doc.pluginId ?? "core", itemType: doc.itemType, itemId: doc.itemId, languageCode: doc.languageCode ?? "", @@ -186,8 +298,26 @@ export class SearchModel { return this.c.get("core").search.adapter; } + /** + * Fills in the document's owner, once, for every write path. + * + * The request's plugin is only a *fallback*: it is the owner when a mutation + * route indexes its own content, and it is `@vitnode/core` during a rebuild, + * which runs inside the core cron request. So an explicit `pluginId` always + * wins - that is how a rebuild reproduces the same ownership a live write + * produced. Resolving it here rather than in each adapter is what keeps the + * canonical row and the mirrored document from disagreeing. + */ + private resolveOwner(doc: SearchDocument): SearchDocument { + return { + ...doc, + pluginId: + searchDocumentOwner(doc.pluginId) ?? this.c.get("plugin")?.id ?? "core", + }; + } + private async upsertRow(doc: SearchDocument): Promise { - const row = { ...toRow(doc), pluginId: this.c.get("plugin")?.id ?? "core" }; + const row = toRow(doc); await this.c .get("db") @@ -200,6 +330,9 @@ export class SearchModel { core_search_index.languageCode, ], set: { + // Included so a rebuild corrects the owner of a row written before the + // indexer declared one, rather than leaving the first writer's guess. + pluginId: row.pluginId, authorId: row.authorId, title: row.title, content: row.content, @@ -216,10 +349,9 @@ export class SearchModel { } async bulkIndex(docs: SearchDocument[]): Promise { - const clean = docs.map(doc => ({ - ...doc, - content: stripHtml(doc.content), - })); + const clean = docs.map(doc => + this.resolveOwner({ ...doc, content: stripHtml(doc.content) }), + ); for (const doc of clean) { await this.upsertRow(doc); @@ -253,7 +385,10 @@ export class SearchModel { /** Canonical projection lives in `core_search_index`; the provider mirrors it. */ async index(doc: SearchDocument): Promise { - const clean = { ...doc, content: stripHtml(doc.content) }; + const clean = this.resolveOwner({ + ...doc, + content: stripHtml(doc.content), + }); await this.upsertRow(clean); await this.provider().index(this.c, clean); diff --git a/packages/vitnode/src/api/modules/admin/debug/debug.admin.module.ts b/packages/vitnode/src/api/modules/admin/debug/debug.admin.module.ts index bf3a6e73b..dc77d8d7d 100644 --- a/packages/vitnode/src/api/modules/admin/debug/debug.admin.module.ts +++ b/packages/vitnode/src/api/modules/admin/debug/debug.admin.module.ts @@ -1,5 +1,6 @@ import { CONFIG_PLUGIN } from "../../../../config"; import { buildModule } from "../../../lib/module"; +import { clearSearchDebugAdminRoute } from "./routes/clear-search.route"; import { integrationsDebugAdminRoute } from "./routes/integrations.route"; import { logsDebugAdminRoute } from "./routes/logs.route"; import { queueDebugAdminRoute } from "./routes/queue.route"; @@ -18,6 +19,7 @@ export const debugAdminModule = buildModule({ queueDebugAdminRoute, searchStatusDebugAdminRoute, rebuildSearchDebugAdminRoute, + clearSearchDebugAdminRoute, sendTestEmailDebugAdminRoute, testAiDebugAdminRoute, testStorageUploadDebugAdminRoute, diff --git a/packages/vitnode/src/api/modules/admin/debug/routes/clear-search.route.ts b/packages/vitnode/src/api/modules/admin/debug/routes/clear-search.route.ts new file mode 100644 index 000000000..404097a12 --- /dev/null +++ b/packages/vitnode/src/api/modules/admin/debug/routes/clear-search.route.ts @@ -0,0 +1,89 @@ +import { HTTPException } from "hono/http-exception"; +import { z } from "zod"; + +import { buildRoute } from "@/api/lib/route"; +import { CONFIG_PLUGIN } from "@/config"; + +export const zodClearSearchSchema = z.object({ + itemType: z.string().min(1), +}); + +/** + * Deletes the documents of one collection that has no registered rebuild + * indexer. + * + * Deliberately not part of `/search/rebuild`: this removes documents and puts + * nothing back, so it must not hide behind an action called "reindex". It is + * refused for a collection that *does* have an indexer - that one has a rebuild, + * which is the non-destructive way to get the same freshness. + * + * What it does **not** mean is that the collection is abandoned. Registering an + * indexer is optional, and a plugin that writes through `search.index()` keeps + * its collection current without one - so a cleared collection can reappear on + * that plugin's next write. This clears the current indexed state; it does not + * stop anything from writing again. + * + * `itemType` is required and non-empty, so there is no payload that clears the + * whole index by omission. A full rebuild is the only thing that does that, and + * it refills what it can. + */ +export const clearSearchDebugAdminRoute = buildRoute({ + pluginId: CONFIG_PLUGIN.pluginId, + adminStaffPermission: { module: "system", permission: "can_view" }, + route: { + method: "post", + description: + "Permanently remove the currently indexed documents of one collection that has no registered rebuild indexer.", + path: "/search/clear", + request: { + body: { + required: true, + content: { + "application/json": { + schema: zodClearSearchSchema, + }, + }, + }, + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ cleared: z.boolean() }), + }, + }, + description: "Collection cleared", + }, + 409: { + description: "The collection has a registered rebuild indexer", + }, + }, + }, + handler: async c => { + const { itemType } = c.req.valid("json"); + + if (c.get("core").searchIndexers.some(i => i.itemType === itemType)) { + throw new HTTPException(409, { + message: `"${itemType}" has a registered rebuild indexer. Rebuild it instead of deleting its documents.`, + }); + } + + await c.get("search").clear(itemType); + + // The documents are already gone, so the audit trail is best effort: the + // logger writes to the database and can fail on its own, and reporting a + // failed cleanup for a cleanup that happened would send an administrator + // looking for documents that are not there. + const message = `[Search] Removed the indexed documents of unmanaged collection "${itemType}".`; + try { + await c.get("log").warn(message); + } catch { + // eslint-disable-next-line no-console + console.warn( + `[VitNode] Failed to persist search cleanup audit: ${message}`, + ); + } + + return c.json({ cleared: true }); + }, +}); diff --git a/packages/vitnode/src/api/modules/admin/debug/routes/rebuild-search.route.ts b/packages/vitnode/src/api/modules/admin/debug/routes/rebuild-search.route.ts index 48bbeebd3..7e8f0516f 100644 --- a/packages/vitnode/src/api/modules/admin/debug/routes/rebuild-search.route.ts +++ b/packages/vitnode/src/api/modules/admin/debug/routes/rebuild-search.route.ts @@ -1,3 +1,4 @@ +import { HTTPException } from "hono/http-exception"; import { z } from "zod"; import { buildRoute } from "@/api/lib/route"; @@ -35,11 +36,24 @@ export const rebuildSearchDebugAdminRoute = buildRoute({ }, description: "Rebuild queued", }, + 404: { description: "No indexer is registered for that collection" }, }, }, handler: async c => { const { itemType } = c.req.valid("json") ?? {}; + // A rebuild of a collection with no indexer would clear it and refill + // nothing, so it is refused here rather than queued and discovered later. + // The task repeats the check for callers that bypass this route. + if ( + itemType && + !c.get("core").searchIndexers.some(i => i.itemType === itemType) + ) { + throw new HTTPException(404, { + message: `No search indexer is registered for "${itemType}".`, + }); + } + await c.get("queue").dispatch({ name: "rebuild-search-index", payload: itemType ? { itemType } : {}, diff --git a/packages/vitnode/src/api/modules/admin/debug/routes/search-collections.route.test.ts b/packages/vitnode/src/api/modules/admin/debug/routes/search-collections.route.test.ts new file mode 100644 index 000000000..4f9bdc463 --- /dev/null +++ b/packages/vitnode/src/api/modules/admin/debug/routes/search-collections.route.test.ts @@ -0,0 +1,279 @@ +// @vitest-environment node +import { HTTPException } from "hono/http-exception"; +import { describe, expect, it, vi } from "vitest"; + +import type { SearchIndexerConfig } from "@/api/models/search"; + +import { clearSearchDebugAdminRoute } from "./clear-search.route"; +import { rebuildSearchDebugAdminRoute } from "./rebuild-search.route"; + +const indexer = (itemType: string): SearchIndexerConfig => ({ + itemType, + load: async () => await Promise.resolve({ documents: [], itemsRead: 0 }), + pluginId: "@vitnode/example", +}); + +interface Dispatched { + name: string; + payload?: Record; +} + +/** + * Both handlers are called directly, the way the queue-task tests call theirs: + * `Route.handler` is deliberately erased, and neither route needs Hono for + * anything but reading a body these tests can hand it outright. + */ +const harness = ({ + body, + clearFails = false, + indexers = [], + logFails = false, +}: { + body?: Record; + clearFails?: boolean; + indexers?: SearchIndexerConfig[]; + logFails?: boolean; +} = {}) => { + const dispatched: Dispatched[] = []; + const cleared: (string | undefined)[] = []; + const warnings: string[] = []; + + const clear = vi.fn(async (itemType?: string) => { + if (clearFails) throw new Error("engine unavailable"); + cleared.push(itemType); + await Promise.resolve(); + }); + + const c = { + get: (key: string) => { + if (key === "core") return { searchIndexers: indexers }; + if (key === "queue") { + return { + dispatch: async (task: Dispatched) => { + dispatched.push(task); + await Promise.resolve(); + }, + }; + } + if (key === "search") { + return { + clear: clear as unknown, + }; + } + if (key === "log") { + return { + warn: async (content: string) => { + warnings.push(content); + await Promise.resolve(); + if (logFails) throw new Error("core_logs unavailable"); + }, + }; + } + + return undefined; + }, + json: (value: unknown) => new Response(JSON.stringify(value)), + req: { valid: () => body }, + }; + + return { c, clear, cleared, dispatched, warnings }; +}; + +const statusOf = async (run: () => Promise) => { + try { + await run(); + } catch (error) { + if (error instanceof HTTPException) return error.status; + throw error; + } + + return 200; +}; + +describe("POST /search/rebuild", () => { + it("queues a scoped rebuild for a registered collection", async () => { + const { c, dispatched } = harness({ + body: { itemType: "example.article" }, + indexers: [indexer("example.article")], + }); + + await rebuildSearchDebugAdminRoute.handler(c); + + expect(dispatched).toEqual([ + { + name: "rebuild-search-index", + payload: { itemType: "example.article" }, + }, + ]); + }); + + it("rejects a scoped rebuild for a collection with no indexer", async () => { + // Queuing this would clear the collection and refill nothing, so the button + // that offers it must fail before anything is dispatched. + const { c, dispatched } = harness({ + body: { itemType: "removed.collection" }, + indexers: [indexer("example.article")], + }); + + await expect( + statusOf(async () => await rebuildSearchDebugAdminRoute.handler(c)), + ).resolves.toBe(404); + expect(dispatched).toEqual([]); + }); + + it("queues a full rebuild with no item type", async () => { + const { c, dispatched } = harness({ + indexers: [indexer("example.article")], + }); + + await rebuildSearchDebugAdminRoute.handler(c); + + expect(dispatched).toEqual([{ name: "rebuild-search-index", payload: {} }]); + }); + + it("queues a full rebuild even with no indexers at all", async () => { + // The guard is about *scoped* rebuilds; a full one is allowed to clear an + // index it cannot fully refill, which is how documents with no indexer get + // removed. + const { c, dispatched } = harness({ body: {} }); + + await rebuildSearchDebugAdminRoute.handler(c); + + expect(dispatched).toHaveLength(1); + }); +}); + +describe("POST /search/clear", () => { + it("clears only the requested collection", async () => { + const { c, cleared, warnings } = harness({ + body: { itemType: "removed.collection" }, + indexers: [indexer("example.article")], + }); + + await clearSearchDebugAdminRoute.handler(c); + + expect(cleared).toEqual(["removed.collection"]); + expect(warnings[0]).toContain("removed.collection"); + }); + + it("refuses a collection that still has an indexer", async () => { + // That one has a rebuild, which gets the same freshness without deleting. + const { c, cleared } = harness({ + body: { itemType: "example.article" }, + indexers: [indexer("example.article")], + }); + + await expect( + statusOf(async () => await clearSearchDebugAdminRoute.handler(c)), + ).resolves.toBe(409); + expect(cleared).toEqual([]); + }); + + it("never clears the whole index", async () => { + // `itemType` is required and non-empty in the schema, so there is no payload + // that reaches `clear(undefined)` through this route. + const { c, cleared } = harness({ + body: { itemType: "removed.collection" }, + }); + + await clearSearchDebugAdminRoute.handler(c); + + expect(cleared).not.toContain(undefined); + }); + + it("writes a neutral audit warning", async () => { + const { c, warnings } = harness({ + body: { itemType: "live.only" }, + }); + + await clearSearchDebugAdminRoute.handler(c); + + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("unmanaged collection"); + expect(warnings[0]).toContain("live.only"); + // Nothing claims the plugin is gone: registering an indexer is optional. + expect(warnings[0]).not.toContain("orphan"); + }); + + it("stays successful when the audit log fails", async () => { + // The documents are already gone. Reporting a failure would send an + // administrator looking for documents that are not there. + const consoleWarn = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + + try { + const { c, clear, cleared, warnings } = harness({ + body: { itemType: "live.only" }, + logFails: true, + }); + + const res = await clearSearchDebugAdminRoute.handler(c); + + await expect(new Response(res.body).json()).resolves.toEqual({ + cleared: true, + }); + expect(cleared).toEqual(["live.only"]); + // Attempted once, and not retried because the log failed. + expect(clear).toHaveBeenCalledTimes(1); + expect(warnings).toHaveLength(1); + expect(consoleWarn).toHaveBeenCalledTimes(1); + expect(String(consoleWarn.mock.calls[0][0])).toContain( + "Failed to persist search cleanup audit", + ); + } finally { + consoleWarn.mockRestore(); + } + }); + + it("does not reach the console when the audit log succeeds", async () => { + const consoleWarn = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + + try { + const { c } = harness({ body: { itemType: "live.only" } }); + + await clearSearchDebugAdminRoute.handler(c); + + expect(consoleWarn).not.toHaveBeenCalled(); + } finally { + consoleWarn.mockRestore(); + } + }); + + it("propagates a failed clear and writes no success audit", async () => { + const { c, warnings } = harness({ + body: { itemType: "live.only" }, + clearFails: true, + }); + + await expect(clearSearchDebugAdminRoute.handler(c)).rejects.toThrow( + "engine unavailable", + ); + expect(warnings).toEqual([]); + }); + + it("rejects an empty item type at the schema", () => { + expect( + zodBody(clearSearchDebugAdminRoute).safeParse({ itemType: "" }).success, + ).toBe(false); + expect(zodBody(clearSearchDebugAdminRoute).safeParse({}).success).toBe( + false, + ); + expect( + zodBody(clearSearchDebugAdminRoute).safeParse({ itemType: "a.b" }) + .success, + ).toBe(true); + }); +}); + +/** Reaches the body schema the route declared, so the test asserts on the real one. */ +function zodBody(route: typeof clearSearchDebugAdminRoute) { + const body = route.route.request?.body; + if (!body || !("content" in body)) throw new Error("No body schema."); + + return body.content["application/json"].schema as { + safeParse: (value: unknown) => { success: boolean }; + }; +} diff --git a/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.test.ts b/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.test.ts new file mode 100644 index 000000000..0dc884621 --- /dev/null +++ b/packages/vitnode/src/api/modules/admin/debug/routes/search-status.route.test.ts @@ -0,0 +1,263 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; + +import type { SearchIndexerConfig } from "@/api/models/search"; + +import { searchStatusDebugAdminRoute } from "./search-status.route"; + +interface IndexedRow { + indexed: number; + itemType: string; + lastIndexedAt: Date | null; + pluginId: null | string; +} + +interface Collection { + hasIndexer: boolean; + indexed: number; + itemType: string; + pluginId: string; + total: null | number; +} + +const indexer = ( + itemType: string, + pluginId: string, + total?: number, +): SearchIndexerConfig => ({ + itemType, + ...(total === undefined + ? {} + : { count: async () => await Promise.resolve(total) }), + load: async () => await Promise.resolve({ documents: [], itemsRead: 0 }), + pluginId, +}); + +/** + * The handler is called directly, the way the queue-task tests call theirs: it + * takes no request input, so routing it through Hono would only add a cast - + * `Route.handler` is deliberately erased to `(...args: unknown[])`. + * + * The stub answers the coverage query with the given rows and every later query + * (the sync-error panel) with nothing. + */ +const harness = ({ + indexers = [], + rows = [], +}: { + indexers?: SearchIndexerConfig[]; + rows?: IndexedRow[]; +} = {}) => { + const results: unknown[][] = [rows, []]; + let call = 0; + + const chain = (value: unknown[]) => { + const builder: Record = {}; + for (const op of ["from", "groupBy", "where", "orderBy", "limit"]) { + builder[op] = () => builder; + } + builder.then = async (resolve: (rows: unknown[]) => TResult) => + await Promise.resolve(value).then(resolve); + + return builder; + }; + + const db = { + select: () => { + const value = results[call] ?? []; + call++; + + return chain(value); + }, + }; + + let body: undefined | { collections: Collection[] }; + + const c = { + get: (key: string) => { + if (key === "db") return db; + if (key === "search") { + return { + name: () => "postgres", + ping: async () => await Promise.resolve(true), + }; + } + if (key === "core") { + return { hasCronAdapter: true, searchIndexers: indexers }; + } + + return undefined; + }, + json: (value: { collections: Collection[] }) => { + body = value; + + return new Response(); + }, + }; + + return { + collections: async (): Promise => { + await searchStatusDebugAdminRoute.handler(c); + + if (!body) throw new Error("The handler returned no body."); + + return body.collections; + }, + }; +}; + +const indexedRow = ( + itemType: string, + pluginId: null | string, + indexed: number, +): IndexedRow => ({ indexed, itemType, lastIndexedAt: null, pluginId }); + +describe("search status collection ownership", () => { + it("uses the registered indexer's plugin", async () => { + const { collections } = harness({ + indexers: [indexer("example.article", "@vitnode/example", 3)], + rows: [indexedRow("example.article", "@vitnode/example", 3)], + }); + + await expect(collections()).resolves.toEqual([ + expect.objectContaining({ + hasIndexer: true, + indexed: 3, + itemType: "example.article", + pluginId: "@vitnode/example", + total: 3, + }), + ]); + }); + + it("falls back to the stored owner when no indexer is registered", async () => { + // No rebuild indexer, for any reason - never registered, or its plugin is + // gone - but the rows still say who wrote them. Reassigning them to core + // would be a lie. + const { collections } = harness({ + rows: [indexedRow("example.article", "@vitnode/example", 3)], + }); + + const [collection] = await collections(); + + expect(collection.pluginId).toBe("@vitnode/example"); + expect(collection.hasIndexer).toBe(false); + // No indexer to ask for a source count, so there is none to report. + expect(collection.total).toBeNull(); + }); + + it("reports `unknown` when neither source names an owner", async () => { + const { collections } = harness({ + rows: [indexedRow("mystery.item", null, 2)], + }); + + const [collection] = await collections(); + + expect(collection.pluginId).toBe("unknown"); + }); + + it("treats a blank stored owner as unknown", async () => { + const { collections } = harness({ + rows: [indexedRow("mystery.item", " ", 2)], + }); + + const [collection] = await collections(); + + expect(collection.pluginId).toBe("unknown"); + }); + + it("keeps the registered owner when the stored one disagrees", async () => { + // The next rebuild rewrites the rows, so the live indexer is canonical. + const { collections } = harness({ + indexers: [indexer("example.article", "@vitnode/example", 3)], + rows: [indexedRow("example.article", "@vitnode/old-example", 3)], + }); + + const [collection] = await collections(); + + expect(collection.pluginId).toBe("@vitnode/example"); + }); + + it("keeps an over-indexed collection with no indexer truthful", async () => { + // The regression: `total` used to fall back to `indexed`, so a collection + // with no indexer reported 11/11 and read as fully indexed. There is no + // source to count, so there is no total - and it must still not be called + // core. + const { collections } = harness({ + rows: [indexedRow("example.article", "@vitnode/example", 11)], + }); + + await expect(collections()).resolves.toEqual([ + expect.objectContaining({ + hasIndexer: false, + indexed: 11, + pluginId: "@vitnode/example", + total: null, + }), + ]); + }); + + it("reports the real counts of an over-indexed registered collection", async () => { + const { collections } = harness({ + indexers: [indexer("example.article", "@vitnode/example", 9)], + rows: [indexedRow("example.article", "@vitnode/example", 11)], + }); + + const [collection] = await collections(); + + // Neither number is rewritten to hide the extra documents, so the AdminCP + // still reads this as stale. + expect(collection).toMatchObject({ + hasIndexer: true, + indexed: 11, + pluginId: "@vitnode/example", + total: 9, + }); + }); + + it("lists a registered collection with nothing indexed yet", async () => { + const { collections } = harness({ + indexers: [indexer("example.article", "@vitnode/example", 5)], + }); + + await expect(collections()).resolves.toEqual([ + expect.objectContaining({ + hasIndexer: true, + indexed: 0, + pluginId: "@vitnode/example", + total: 5, + }), + ]); + }); + + it("reports an indexer with no `count` against the indexed total", async () => { + // `count` is optional, and leaving it out is documented as "assume covered". + // That is still a registered collection, so `hasIndexer` stays true. + const { collections } = harness({ + indexers: [indexer("example.article", "@vitnode/example")], + rows: [indexedRow("example.article", "@vitnode/example", 4)], + }); + + const [collection] = await collections(); + + expect(collection).toMatchObject({ + hasIndexer: true, + indexed: 4, + total: 4, + }); + }); + + it("does not infer an indexer from a stored owner", async () => { + // The rule the field exists for: rows knowing who wrote them says nothing + // about whether a rebuild indexer is registered. It says nothing the other + // way either - a plugin may be installed, active, and writing live. + const { collections } = harness({ + rows: [indexedRow("example.article", "@vitnode/example", 3)], + }); + + const [collection] = await collections(); + + expect(collection.pluginId).toBe("@vitnode/example"); + expect(collection.hasIndexer).toBe(false); + }); +}); 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 629145a46..ef2778c9d 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,16 +1,36 @@ -import { countDistinct, max } from "drizzle-orm"; +import { and, countDistinct, desc, eq, like, max } from "drizzle-orm"; import { z } from "zod"; import { buildRoute } from "@/api/lib/route"; +import { searchDocumentOwner } from "@/api/models/search"; import { CONFIG_PLUGIN } from "@/config"; +import { core_logs } from "@/database/logs"; import { core_search_index } from "@/database/search"; +const CONTENT_SEARCH_LOG_PREFIX = "[content-search]"; + +const SYNC_ERROR_LIMIT = 10; + const collectionSchema = z.object({ + /** + * 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 + * simply not loaded in this process. + */ + hasIndexer: z.boolean(), indexed: z.number(), itemType: z.string(), lastIndexedAt: z.date().nullable(), pluginId: z.string(), - total: z.number(), + /** Source items the indexer reports. `null` when there is no indexer to ask. */ + total: z.number().nullable(), +}); + +const syncErrorSchema = z.object({ + content: z.string(), + createdAt: z.date(), + id: z.number(), + pluginId: z.string(), }); export const searchStatusDebugAdminRoute = buildRoute({ @@ -31,6 +51,7 @@ export const searchStatusDebugAdminRoute = buildRoute({ hasCronAdapter: z.boolean(), healthy: z.boolean(), lastIndexedAt: z.date().nullable(), + syncErrors: z.array(syncErrorSchema), total: z.number(), }), }, @@ -46,17 +67,42 @@ export const searchStatusDebugAdminRoute = buildRoute({ // One item can emit several index rows (e.g. one per language), so coverage // is measured in distinct items - not documents. + // + // `pluginId` comes along so a collection whose indexer is gone can still name + // its owner. An item type has one owner, so the aggregate is a formality - + // `max` picks deterministically if rows ever disagree mid-rebuild. const indexedByType = await db .select({ itemType: core_search_index.itemType, indexed: countDistinct(core_search_index.itemId), lastIndexedAt: max(core_search_index.indexedAt), + pluginId: max(core_search_index.pluginId), }) .from(core_search_index) .groupBy(core_search_index.itemType); const statsByType = new Map(indexedByType.map(row => [row.itemType, row])); + // 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 `_`. + const syncErrors = await db + .select({ + id: core_logs.id, + pluginId: core_logs.pluginId, + content: core_logs.content, + createdAt: core_logs.createdAt, + }) + .from(core_logs) + .where( + and( + eq(core_logs.type, "error"), + like(core_logs.content, `${CONTENT_SEARCH_LOG_PREFIX}%`), + ), + ) + .orderBy(desc(core_logs.id)) + .limit(SYNC_ERROR_LIMIT); + // Start from every registered indexer so a collection with nothing indexed // yet still appears; then fold in any indexed type without a live indexer. const itemTypes = [ @@ -71,15 +117,32 @@ export const searchStatusDebugAdminRoute = buildRoute({ const indexer = core.searchIndexers.find(i => i.itemType === itemType); const stats = statsByType.get(itemType); const indexed = stats?.indexed ?? 0; - const total = indexer?.count ? await indexer.count(c) : indexed; + // No indexer, no source count - and inventing `total = indexed` would + // report a collection nothing can rebuild as fully covered. An indexer + // without the optional `count` still falls back to the indexed count, + // which is the documented behaviour of leaving `count` out. + // + // `hasIndexer` says only whether a rebuild indexer exists. A plugin may + // keep its collection current through `search.index()` and register none, + // so this is not a statement about the plugin. + const total = indexer ? ((await indexer.count?.(c)) ?? indexed) : null; return { + hasIndexer: indexer !== undefined, itemType, - pluginId: indexer?.pluginId ?? "core", + // The registered indexer is canonical - it is what the next rebuild + // will stamp on the rows. Falling back to the stored owner is what + // stops a collection with no indexer being reassigned to core, and + // `"unknown"` is honest when neither source knows. + pluginId: + indexer?.pluginId ?? + searchDocumentOwner(stats?.pluginId) ?? + "unknown", indexed, - // A source count below the indexed count (e.g. items deleted since the - // last rebuild) would break the coverage bar; never report less. - total: Math.max(total, indexed), + // 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. + total, lastIndexedAt: stats?.lastIndexedAt ?? null, }; }), @@ -97,6 +160,7 @@ export const searchStatusDebugAdminRoute = buildRoute({ hasCronAdapter: core.hasCronAdapter, healthy: await search.ping(), lastIndexedAt, + syncErrors, total: collections.reduce((sum, row) => sum + row.indexed, 0), }); }, diff --git a/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.test.ts b/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.test.ts new file mode 100644 index 000000000..ea7e488be --- /dev/null +++ b/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.test.ts @@ -0,0 +1,518 @@ +// @vitest-environment node +import type { Context } from "hono"; + +import { describe, expect, it, vi } from "vitest"; + +import type { EnvVitNode } from "@/api/middlewares/global.middleware"; +import type { + SearchDocument, + SearchIndexerConfig, + SearchIndexerLoadResult, +} from "@/api/models/search"; + +import { rebuildSearchIndexTask } from "./rebuild-index.task"; + +const document = ( + itemType: string, + itemId: number, + extra: Partial = {}, +): SearchDocument => ({ + content: "body", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + itemId, + itemType, + title: `Item ${itemId}`, + ...extra, +}); + +/** + * An indexer that replays a fixed list of pages, recording the offsets it was + * asked for. Page contents are what each test is about; the offsets are what the + * rebuild is supposed to derive from them. + */ +const scriptedIndexer = ({ + itemType, + pages, + pluginId, +}: { + itemType: string; + /** + * Either result shape, so the same script drives a modern and a legacy + * indexer. Past the end, each keeps returning its own "no more rows" signal. + */ + pages: SearchIndexerLoadResult[]; + pluginId: string; +}) => { + const offsets: number[] = []; + const exhausted: SearchIndexerLoadResult = Array.isArray(pages[0]) + ? [] + : { documents: [], itemsRead: 0 }; + let call = 0; + + const config: SearchIndexerConfig = { + itemType, + load: async (_c, offset) => { + offsets.push(offset); + const page = pages[call] ?? exhausted; + call++; + + return await Promise.resolve(page); + }, + pluginId, + }; + + return { config, offsets }; +}; + +const harness = (indexers: SearchIndexerConfig[]) => { + const cleared: (string | undefined)[] = []; + const indexed: SearchDocument[][] = []; + + const search = { + bulkIndex: vi.fn(async (docs: SearchDocument[]) => { + indexed.push(docs); + await Promise.resolve(); + }), + clear: vi.fn(async (itemType?: string) => { + cleared.push(itemType); + await Promise.resolve(); + }), + }; + + const c = { + get: (key: string) => { + if (key === "search") return search; + if (key === "core") return { searchIndexers: indexers }; + + return undefined; + }, + } as unknown as Context; + + return { c, cleared, indexed, search }; +}; + +describe("rebuild-search-index", () => { + it("keeps paging after a page that produced no documents", async () => { + // The regression: every row on page 1 is rejected by the mapper. Treating an + // empty document array as end-of-source would stop here and never index the + // valid row on page 2. + const { config, offsets } = scriptedIndexer({ + itemType: "test.searchable", + pages: [ + { documents: [], itemsRead: 200 }, + { documents: [document("test.searchable", 201)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(offsets).toEqual([0, 200, 201]); + expect(indexed).toHaveLength(1); + expect(indexed[0]?.[0]?.itemId).toBe(201); + }); + + it("stops when a page reads no source rows", async () => { + const { config, offsets } = scriptedIndexer({ + itemType: "test.searchable", + pages: [ + { documents: [document("test.searchable", 1)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(offsets).toEqual([0, 1]); + expect(indexed).toHaveLength(1); + }); + + it("never calls the engine for an empty document page", async () => { + const { config } = scriptedIndexer({ + itemType: "test.searchable", + pages: [ + { documents: [], itemsRead: 5 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed).toEqual([]); + }); + + it("advances by source items, not by documents", async () => { + // A multi-language indexer emits several documents per item. Advancing by + // document count would skip items on every page. + const { config, offsets } = scriptedIndexer({ + itemType: "blog_post", + pages: [ + { + documents: [ + document("blog_post", 1, { languageCode: "en" }), + document("blog_post", 1, { languageCode: "pl" }), + document("blog_post", 2, { languageCode: "en" }), + document("blog_post", 2, { languageCode: "pl" }), + ], + itemsRead: 2, + }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/blog", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(offsets).toEqual([0, 2]); + expect(indexed[0]).toHaveLength(4); + }); + + describe("plugin ownership", () => { + it("keeps an owner the document already declared", async () => { + const { config } = scriptedIndexer({ + itemType: "test.searchable", + pages: [ + { + documents: [ + document("test.searchable", 1, { pluginId: "@vitnode/example" }), + ], + itemsRead: 1, + }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed[0]?.[0]?.pluginId).toBe("@vitnode/example"); + }); + + it("stamps the registering plugin on a legacy document", async () => { + // A hand-written indexer that predates `SearchDocument.pluginId`. Without + // this the rebuild would store it as core, because the queue drains inside + // the core cron request. + const { config } = scriptedIndexer({ + itemType: "blog_post", + pages: [ + { documents: [document("blog_post", 7)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/blog", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed[0]?.[0]?.pluginId).toBe("@vitnode/blog"); + }); + + it("preserves ownership in a single-collection rebuild", async () => { + const example = scriptedIndexer({ + itemType: "test.searchable", + pages: [ + { + documents: [ + document("test.searchable", 1, { pluginId: "@vitnode/example" }), + ], + itemsRead: 1, + }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const blog = scriptedIndexer({ + itemType: "blog_post", + pages: [{ documents: [document("blog_post", 1)], itemsRead: 1 }], + pluginId: "@vitnode/blog", + }); + const { c, cleared, indexed } = harness([example.config, blog.config]); + + await rebuildSearchIndexTask.handler(c, { + itemType: "test.searchable", + }); + + // Scoped: the other plugin's collection is neither cleared nor reloaded. + expect(cleared).toEqual(["test.searchable"]); + expect(blog.offsets).toEqual([]); + expect(indexed.flat().map(doc => doc.pluginId)).toEqual([ + "@vitnode/example", + ]); + }); + }); + + it("clears the whole index for a full rebuild and each indexer runs", async () => { + const first = scriptedIndexer({ + itemType: "a.one", + pages: [ + { documents: [document("a.one", 1)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/a", + }); + const second = scriptedIndexer({ + itemType: "b.two", + pages: [ + { documents: [document("b.two", 1)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/b", + }); + const { c, cleared, indexed } = harness([first.config, second.config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(cleared).toEqual([undefined]); + expect(indexed.flat().map(doc => doc.pluginId)).toEqual([ + "@vitnode/a", + "@vitnode/b", + ]); + }); + + describe("the deprecated array result", () => { + it("indexes a legacy page and stops on the empty one", async () => { + const { config, offsets } = scriptedIndexer({ + itemType: "legacy.item", + pages: [[document("legacy.item", 1)], []], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + // No source count to advance by, so the cursor moves a whole page - which + // is exactly what the old rebuild did. + expect(offsets).toEqual([0, 200]); + expect(indexed).toHaveLength(1); + expect(indexed[0]?.[0]?.itemId).toBe(1); + }); + + it("stamps the registering plugin on a legacy document", async () => { + const { config } = scriptedIndexer({ + itemType: "legacy.item", + pages: [[document("legacy.item", 1)], []], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed[0]?.[0]?.pluginId).toBe("@vitnode/legacy"); + }); + + it("keeps an owner a legacy document declared itself", async () => { + const { config } = scriptedIndexer({ + itemType: "legacy.item", + pages: [ + [document("legacy.item", 1, { pluginId: "@vitnode/elsewhere" })], + [], + ], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed[0]?.[0]?.pluginId).toBe("@vitnode/elsewhere"); + }); + + it("treats a blank declared owner as absent", async () => { + const { config } = scriptedIndexer({ + itemType: "legacy.item", + pages: [[document("legacy.item", 1, { pluginId: " " })], []], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(indexed[0]?.[0]?.pluginId).toBe("@vitnode/legacy"); + }); + + it("does not mistake a multilingual document count for a source count", async () => { + // Four documents, two source items, two languages. Advancing by the array + // length would jump to offset 4 and skip most of a 200-row page. + const { config, offsets } = scriptedIndexer({ + itemType: "legacy.multilingual", + pages: [ + [ + document("legacy.multilingual", 1, { languageCode: "en" }), + document("legacy.multilingual", 1, { languageCode: "pl" }), + document("legacy.multilingual", 2, { languageCode: "en" }), + document("legacy.multilingual", 2, { languageCode: "pl" }), + ], + [], + ], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(offsets).toEqual([0, 200]); + expect(offsets).not.toContain(4); + expect(indexed[0]).toHaveLength(4); + }); + + it("indexes an empty first page as an exhausted source", async () => { + // The ambiguity the modern contract exists to remove: an array cannot say + // whether rows were read and all filtered out, so this ends the rebuild. + const { config, offsets } = scriptedIndexer({ + itemType: "legacy.item", + pages: [[], [document("legacy.item", 1)]], + pluginId: "@vitnode/legacy", + }); + const { c, indexed } = harness([config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(offsets).toEqual([0]); + expect(indexed).toEqual([]); + }); + + it("runs alongside a modern indexer", async () => { + const legacy = scriptedIndexer({ + itemType: "legacy.item", + pages: [[document("legacy.item", 1)], []], + pluginId: "@vitnode/legacy", + }); + const modern = scriptedIndexer({ + itemType: "modern.item", + pages: [ + { documents: [], itemsRead: 200 }, + { documents: [document("modern.item", 2)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/modern", + }); + const { c, indexed } = harness([legacy.config, modern.config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(legacy.offsets).toEqual([0, 200]); + expect(modern.offsets).toEqual([0, 200, 201]); + expect(indexed.flat().map(doc => [doc.itemId, doc.pluginId])).toEqual([ + [1, "@vitnode/legacy"], + [2, "@vitnode/modern"], + ]); + }); + }); + + describe("a collection with no rebuild indexer", () => { + it("refuses a scoped rebuild before clearing anything", async () => { + // The action offering this is called "reindex", so it must not be a delete: + // clearing here would remove the documents and refill nothing. + const other = scriptedIndexer({ + itemType: "example.article", + pages: [{ documents: [], itemsRead: 0 }], + pluginId: "@vitnode/example", + }); + const { c, indexed, search } = harness([other.config]); + + await expect( + rebuildSearchIndexTask.handler(c, { itemType: "removed.collection" }), + ).rejects.toThrow(/no search indexer is registered/i); + + expect(search.clear).not.toHaveBeenCalled(); + expect(other.offsets).toEqual([]); + expect(indexed).toEqual([]); + }); + + it("refuses even when no indexer is registered at all", async () => { + const { c, search } = harness([]); + + await expect( + rebuildSearchIndexTask.handler(c, { itemType: "removed.collection" }), + ).rejects.toThrow(/removed.collection/); + + expect(search.clear).not.toHaveBeenCalled(); + }); + + it("names the collection it refused", async () => { + const { c } = harness([]); + + await expect( + rebuildSearchIndexTask.handler(c, { itemType: "removed.collection" }), + ).rejects.toThrow(/Cannot rebuild collection "removed.collection"/); + }); + + it("still lets a full rebuild clear the whole index", async () => { + // A full rebuild refills only what has an indexer, so documents without one + // are removed by it. That is the documented behaviour, and it must not be + // blocked by the scoped guard. + const registered = scriptedIndexer({ + itemType: "example.article", + pages: [ + { documents: [document("example.article", 1)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const { c, cleared, indexed } = harness([registered.config]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(cleared).toEqual([undefined]); + expect(indexed.flat().map(doc => doc.itemId)).toEqual([1]); + }); + + it("still lets a full rebuild run with no indexers registered", async () => { + const { c, cleared, indexed } = harness([]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(cleared).toEqual([undefined]); + expect(indexed).toEqual([]); + }); + }); + + it("clears and rebuilds only the scoped collection when it has an indexer", async () => { + const target = scriptedIndexer({ + itemType: "example.article", + pages: [ + { documents: [document("example.article", 1)], itemsRead: 1 }, + { documents: [], itemsRead: 0 }, + ], + pluginId: "@vitnode/example", + }); + const other = scriptedIndexer({ + itemType: "blog_post", + pages: [{ documents: [document("blog_post", 9)], itemsRead: 1 }], + pluginId: "@vitnode/blog", + }); + const { c, cleared, indexed } = harness([target.config, other.config]); + + await rebuildSearchIndexTask.handler(c, { itemType: "example.article" }); + + expect(cleared).toEqual(["example.article"]); + expect(other.offsets).toEqual([]); + expect(indexed.flat().map(doc => doc.itemId)).toEqual([1]); + }); + + it("does not loop forever on a broken indexer", async () => { + // A page that reports rows but never advances past them would spin. The + // cursor is the indexer's own `itemsRead`, so this asserts the loop is driven + // by data rather than by a fixed page size. + const load = vi.fn( + async (_c: Context, offset: number) => + await Promise.resolve( + offset === 0 + ? { documents: [], itemsRead: 3 } + : { documents: [], itemsRead: 0 }, + ), + ); + const { c } = harness([{ itemType: "x.y", load, pluginId: "@vitnode/x" }]); + + await rebuildSearchIndexTask.handler(c, {}); + + expect(load).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.ts b/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.ts index 03d396dda..ddd07ad58 100644 --- a/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.ts +++ b/packages/vitnode/src/api/modules/search/tasks/rebuild-index.task.ts @@ -1,4 +1,8 @@ import { buildQueueTask } from "@/api/lib/queue"; +import { + normalizeSearchIndexerPage, + searchDocumentOwner, +} from "@/api/models/search"; const PAGE_SIZE = 200; @@ -16,20 +20,50 @@ export const rebuildSearchIndexTask = buildQueueTask({ indexer => !itemType || indexer.itemType === itemType, ); + // Clearing a collection nothing can rebuild is a delete, not a rebuild. The + // route rejects this too, but the check has to be here as well: a queue task + // can be dispatched directly, and a job queued while an indexer was still + // registered can drain after the plugin is gone. + if (itemType && indexers.length === 0) { + throw new Error( + `[Search] Cannot rebuild collection "${itemType}": no search indexer is registered. Its documents were left alone - they may still be maintained by live writes, so remove them explicitly if that is what you meant.`, + ); + } + // Scope the clear to the target collection so a single-collection reindex // never wipes the rest of the index. await search.clear(itemType); for (const indexer of indexers) { - // Offset advances by whole pages of items, not by document count: an - // indexer may emit several documents per item (e.g. one per language), so - // the two are not interchangeable. A page that yields no documents ends - // the loop. - for (let page = 0; ; page++) { - const docs = await indexer.load(c, page * PAGE_SIZE, PAGE_SIZE); - if (docs.length === 0) break; + // The cursor advances by source rows read, never by documents produced: an + // indexer may emit several documents per item, or none for a page whose + // rows it cannot project. Ending on an empty document array would stop the + // rebuild at the first such page and never reach the rows after it. + // + // A legacy indexer returning a bare array is normalized to the same shape, + // where a non-empty page reports the requested limit - the only cursor the + // old contract ever had. + for (let offset = 0; ;) { + const page = normalizeSearchIndexerPage( + await indexer.load(c, offset, PAGE_SIZE), + PAGE_SIZE, + ); + if (page.itemsRead === 0) break; + + if (page.documents.length > 0) { + // This task runs inside the core cron request, so the request's plugin + // is not the owner. Stamp the registering plugin on any document that + // did not name one, or a rebuild would relabel it as core. + await search.bulkIndex( + page.documents.map(document => ({ + ...document, + pluginId: + searchDocumentOwner(document.pluginId) ?? indexer.pluginId, + })), + ); + } - await search.bulkIndex(docs); + offset += page.itemsRead; } } }, diff --git a/packages/vitnode/src/content/const.ts b/packages/vitnode/src/content/const.ts index a325d7e30..22db51cf8 100644 --- a/packages/vitnode/src/content/const.ts +++ b/packages/vitnode/src/content/const.ts @@ -1,16 +1,5 @@ -/** - * Columns the Content Engine always adds. They can never be declared as - * content fields - `defineContentType` rejects them. - */ export const CONTENT_SYSTEM_FIELDS = ["id", "createdAt", "updatedAt"] as const; -/** - * Columns generated by `publication: { enabled: true }`. - * - * Reserved *only* when publication is enabled. Plenty of Stage 1 content types - * declare their own `status` enum - that stays legal, and opting into - * publication is what turns the name into an error. - */ export const CONTENT_PUBLICATION_FIELDS = ["status", "publishedAt"] as const; export const CONTENT_PUBLICATION_STATUSES = ["draft", "published"] as const; @@ -19,35 +8,13 @@ const publicationStatuses: ReadonlySet = new Set( CONTENT_PUBLICATION_STATUSES, ); -/** - * Whether a value is one of the two generated publication statuses. - * - * The generated Zod schemas already narrow this on the HTTP path, so this is - * defence in depth for the direct-service path: a cast, a plain-JavaScript - * caller or an object built at runtime can put anything in `filters.status`, - * and that value must never reach Drizzle. - * - * Takes `unknown` and returns a predicate derived from the constant rather than - * from `ContentPublicationStatus`, so this module stays free of type imports - * from `types.ts` - which imports from here. - */ export const isContentPublicationStatus = ( value: unknown, ): value is (typeof CONTENT_PUBLICATION_STATUSES)[number] => typeof value === "string" && publicationStatuses.has(value); -/** `varchar` length of the generated `status` column. */ export const CONTENT_PUBLICATION_STATUS_LENGTH = 32; -/** - * Field kinds a generated equality filter understands. - * - * `textarea` and `dateTime` are absent on purpose: equality against a body of - * prose, or against one exact timestamp, is never what anyone means. - * - * One list, three consumers - the filter schema, the query builder and the - * public service types all derive from it, so they cannot drift apart. - */ export const CONTENT_FILTERABLE_FIELD_KINDS = [ "boolean", "enum", @@ -62,19 +29,9 @@ const filterableFieldKinds: ReadonlySet = new Set( CONTENT_FILTERABLE_FIELD_KINDS, ); -/** - * Whether a field of this kind may back a generated equality filter. - * - * Takes a plain `string` rather than `ContentFieldKind` so this module stays - * free of type imports from `types.ts`, which imports from here. - */ export const isFilterableFieldKind = (kind: string): boolean => filterableFieldKinds.has(kind); -/** - * Query-string keys owned by pagination and ordering. A filter may not use one - * of these names or it would silently shadow the pagination contract. - */ export const RESERVED_FILTER_KEYS = [ "cursor", "first", @@ -105,11 +62,6 @@ export const CONTENT_CACHE_TAG_MAX_LENGTH = 256; export const CONTENT_TEXT_DEFAULT_LENGTH = 255; export const CONTENT_ENUM_DEFAULT_LENGTH = 64; -/** - * `varchar` length of a slug column, and the length {@link slugify} truncates - * to. Shorter than a text field on purpose: a slug is a URL segment, and 160 - * characters is already far past what anyone types or shares. - */ export const CONTENT_SLUG_DEFAULT_LENGTH = 160; export const CONTENT_DEFAULT_PAGE_SIZE = 25; @@ -118,31 +70,12 @@ export const CONTENT_OPTIONS_LIMIT = 25; export const CONTENT_PUBLIC_DEFAULT_PAGE_SIZE = 25; export const CONTENT_PUBLIC_MAX_PAGE_SIZE = 50; -/** - * URL segment for a public content type: lowercase, dash separated, one - * segment. No slashes, so a leading or trailing one, an empty segment and `..` - * are all rejected by construction rather than by three more checks. - */ export const CONTENT_PUBLIC_PATH_PATTERN = /^[a-z][a-z0-9-]*$/; export const CONTENT_PUBLIC_PATH_MAX_LENGTH = 64; -/** - * Path segments a public content type may not claim. - * - * `admin` is the important one: the global admin gate is a `path.includes( - * "/admin/")` substring test, so a public route under that name would demand a - * staff session and never be public at all. - */ export const CONTENT_PUBLIC_RESERVED_PATHS = ["admin"] as const; -/** - * Field kinds a public response may carry. - * - * `user` is deliberately absent. A user field resolves to a person, and the - * first public layer should not make leaking one a one-word change - expose an - * author through your own route, with the shape you actually mean. - */ export const CONTENT_PUBLIC_EXPOSABLE_KINDS = [ "boolean", "dateTime", @@ -154,12 +87,6 @@ export const CONTENT_PUBLIC_EXPOSABLE_KINDS = [ "textarea", ] as const; -/** - * Generated columns `publicApi.fields` may name. - * - * `status` is missing on purpose: every row the public API returns is - * published, so the column would be a constant. - */ export const CONTENT_PUBLIC_EXPOSABLE_COLUMNS = [ "id", "createdAt", @@ -167,13 +94,40 @@ export const CONTENT_PUBLIC_EXPOSABLE_COLUMNS = [ "publishedAt", ] as const; +export const CONTENT_PUBLIC_ALWAYS_ORDERABLE = "publishedAt"; + +/** + * Field kinds `search.titleField` may name. + * + * `text` only. A search title is one line, weighted `A` by the index; prose from + * a `textarea` in that slot ruins ranking for every other document, and a slug + * is already in the URL. + */ +export const CONTENT_SEARCH_TITLE_KINDS = ["text"] as const; + +/** Field kinds `search.descriptionField` may name. */ +export const CONTENT_SEARCH_DESCRIPTION_KINDS = ["text", "textarea"] as const; + /** - * Always available to `orderBy`, with no entry in `publicApi.orderableFields` - - * the same courtesy the admin list extends to its system columns. It is the - * natural order of a public feed, and it leaks nothing that is not already - * implied by the row being published. + * Field kinds `search.contentFields` may name. + * + * Prose only. `enum`, `number`, `boolean` and `dateTime` are facets, not text: + * full-text-indexing `"draft"` or `"42"` is noise, and it would let a searcher + * probe values. `relation` is a foreign key, and `user` is never public. */ -export const CONTENT_PUBLIC_ALWAYS_ORDERABLE = "publishedAt"; +export const CONTENT_SEARCH_TEXT_KINDS = ["slug", "text", "textarea"] as const; + +/** The only placeholder `search.pathTemplate` may use. */ +export const CONTENT_SEARCH_SLUG_PLACEHOLDER = "{slug}"; + +/** + * `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 + * the definition that caused it. + */ +export const CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH = 100; + +export const CONTENT_SEARCH_PATH_MAX_LENGTH = 512; /** * Every content type gets the first four staff permissions. `can_publish` is diff --git a/packages/vitnode/src/content/define.ts b/packages/vitnode/src/content/define.ts index 39f3232d2..64bd64d8f 100644 --- a/packages/vitnode/src/content/define.ts +++ b/packages/vitnode/src/content/define.ts @@ -7,9 +7,15 @@ import type { ContentPublicApiConfig, ContentPublicationConfig, ContentPublicExposableField, + ContentSearchConfig, + ContentSearchDescriptionField, + ContentSearchEnabled, + ContentSearchTextField, + ContentSearchTitleField, ContentTypeDefinition, ResolvedContentAdminConfig, ResolvedContentPublicApiConfig, + ResolvedContentSearchConfig, } from "./types"; import { @@ -25,6 +31,12 @@ import { CONTENT_PUBLIC_PATH_PATTERN, CONTENT_PUBLIC_RESERVED_PATHS, CONTENT_PUBLICATION_FIELDS, + CONTENT_SEARCH_DESCRIPTION_KINDS, + CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH, + CONTENT_SEARCH_PATH_MAX_LENGTH, + CONTENT_SEARCH_SLUG_PLACEHOLDER, + CONTENT_SEARCH_TEXT_KINDS, + CONTENT_SEARCH_TITLE_KINDS, CONTENT_SYSTEM_FIELDS, CONTENT_TABLE_NAME_PATTERN, isFilterableFieldKind, @@ -553,6 +565,226 @@ const resolvePublicApi = ( }; }; +const searchTitleKinds: ReadonlySet = new Set( + CONTENT_SEARCH_TITLE_KINDS, +); +const searchDescriptionKinds: ReadonlySet = new Set( + CONTENT_SEARCH_DESCRIPTION_KINDS, +); +const searchTextKinds: ReadonlySet = new Set(CONTENT_SEARCH_TEXT_KINDS); + +const disabledSearch: ResolvedContentSearchConfig = { + contentFields: [], + descriptionField: null, + enabled: false, + pathTemplate: "", + titleField: "", +}; + +/** + * Checks one indexed field name. + * + * The public-exposure rule is the important one, and it is checked here as well + * as in the types because a JavaScript caller or a widened value can reach this + * function with anything at all. + */ +const assertSearchField = ({ + exposed, + fields, + id, + kinds, + label, + name, +}: { + exposed: ReadonlySet; + fields: ContentFieldMap; + id: string; + kinds: ReadonlySet; + label: string; + name: string; +}): void => { + const fieldValue = fields[name]; + if (!fieldValue) { + throw new ContentEngineError( + `${label} references unknown field "${name}".`, + { contentTypeId: id }, + ); + } + + if (!kinds.has(fieldValue.kind)) { + throw new ContentEngineError( + `${label} names "${name}" of kind "${fieldValue.kind}". Expected one of: ${[...kinds].sort().join(", ")}.`, + { contentTypeId: id }, + ); + } + + if (!exposed.has(name)) { + throw new ContentEngineError( + `${label} names "${name}", which is not in publicApi.fields. Every indexed field must be publicly exposed - otherwise a result snippet, a highlighted match, ranking or an exact-match probe would leak a private value.`, + { contentTypeId: id }, + ); + } +}; + +const assertSearchPathTemplate = (id: string, template: string): void => { + if (!template.startsWith("/")) { + throw new ContentEngineError( + `search.pathTemplate "${template}" must start with "/". Search result URLs are relative to the site root.`, + { contentTypeId: id }, + ); + } + + if (template.length > CONTENT_SEARCH_PATH_MAX_LENGTH) { + throw new ContentEngineError( + `search.pathTemplate "${template}" is longer than ${CONTENT_SEARCH_PATH_MAX_LENGTH} characters.`, + { contentTypeId: id }, + ); + } + + const occurrences = + template.split(CONTENT_SEARCH_SLUG_PLACEHOLDER).length - 1; + if (occurrences !== 1) { + throw new ContentEngineError( + `search.pathTemplate "${template}" must contain exactly one "${CONTENT_SEARCH_SLUG_PLACEHOLDER}" placeholder, not ${occurrences}.`, + { 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, ""); + 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.`, + { contentTypeId: id }, + ); + } + + if (rest.includes("//") || template.includes("..") || /\s/.test(template)) { + throw new ContentEngineError( + `search.pathTemplate "${template}" must not contain an empty segment, "..", or whitespace.`, + { contentTypeId: id }, + ); + } +}; + +/** + * Checks and fills in `search`. + * + * Runs after `resolvePublicApi`, because every rule here is stated in terms of + * the resolved public allowlist and its single exposed slug field. + */ +const resolveSearch = ( + id: string, + fields: ContentFieldMap, + search: ContentSearchConfig | undefined, + publicApi: ResolvedContentPublicApiConfig, + publication: boolean, +): ResolvedContentSearchConfig => { + if (!search?.enabled) return disabledSearch; + + if (!publication) { + throw new ContentEngineError( + "search needs `publication: { enabled: true }`. Only published records are indexed, and the publication lifecycle is what drives synchronization.", + { contentTypeId: id }, + ); + } + + if (!publicApi.enabled) { + throw new ContentEngineError( + "search needs `publicApi: { enabled: true, path, fields }`. A search hit links to a public URL, and every indexed field has to be published already.", + { contentTypeId: id }, + ); + } + + if (id.length > CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH) { + throw new ContentEngineError( + `Content type id "${id}" is longer than ${CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH} characters, which is the width of the search index's item type column. Shorten the id or turn search off.`, + { contentTypeId: id }, + ); + } + + const exposed = new Set(publicApi.fields); + + const { titleField } = search; + assertSearchField({ + exposed, + fields, + id, + kinds: searchTitleKinds, + label: "search.titleField", + name: titleField, + }); + + // A `null` title can never be a result heading, and a record whose title is + // missing is skipped by the mapper - which shows up as a collection that never + // reaches full coverage. Rejecting the nullable field is the cheap half of + // that; a blank value written straight into the database is still possible, so + // the mapper keeps its own check. + if (fields[titleField].nullable) { + throw new ContentEngineError( + `search.titleField names the nullable field "${titleField}". A search result needs a heading, so the title field must not be nullable.`, + { contentTypeId: id }, + ); + } + + const descriptionField = search.descriptionField ?? null; + if (descriptionField !== null) { + assertSearchField({ + exposed, + fields, + id, + kinds: searchDescriptionKinds, + label: "search.descriptionField", + name: descriptionField, + }); + } + + // `Array.isArray` rather than a cast: a JavaScript caller can put anything + // here, and a `.map` on a bare string would be a TypeError instead of the + // error message below. + const contentFields = Array.isArray(search.contentFields) + ? [...search.contentFields] + : []; + if (contentFields.length === 0) { + throw new ContentEngineError( + "search.contentFields is empty. List at least one field to index - a document with only a title matches almost nothing.", + { contentTypeId: id }, + ); + } + + const duplicate = contentFields.find( + (name, position) => contentFields.indexOf(name) !== position, + ); + if (duplicate !== undefined) { + throw new ContentEngineError( + `search.contentFields lists "${duplicate}" twice.`, + { contentTypeId: id }, + ); + } + + for (const name of contentFields) { + assertSearchField({ + exposed, + fields, + id, + kinds: searchTextKinds, + label: "search.contentFields", + name, + }); + } + + assertSearchPathTemplate(id, search.pathTemplate); + + return { + contentFields, + descriptionField, + enabled: true, + pathTemplate: search.pathTemplate, + titleField, + }; +}; + /** * Declares a content type. The result is plain data - zod and objects only - * so the same definition can be imported by `buildPlugin` (client) and by @@ -565,6 +797,24 @@ export const defineContentType = < TPublication extends boolean = false, TPublicField extends ContentPublicExposableField = never, TPublicEnabled extends boolean = false, + // Inferred from the `search` literal and checked against the public allowlist. + // The constraint is verified once every other parameter is resolved, which is + // what makes "an indexed field is a public field" a compile error. + // The whole `search` argument, inferred as one type. Its *constraint* is what + // enforces the field rules, and a constraint is checked once every other + // parameter is resolved - spelling the same unions out inside the parameter + // type instead lets `TPublicField` fall back to its own constraint while the + // argument is still being checked, and a private field name slips through. + // + // Inferring the object rather than its parts is also what preserves the + // `enabled` literal: an intersection member is not an inference site. + TSearch extends + | ContentSearchConfig< + ContentSearchTitleField, + ContentSearchDescriptionField, + ContentSearchTextField + > + | { enabled: false } = { enabled: false }, >({ admin, fields, @@ -572,6 +822,7 @@ export const defineContentType = < indexes = [], publicApi, publication, + search, tableName, }: { admin: ContentAdminConfig; @@ -586,13 +837,20 @@ export const defineContentType = < ContentPublicApiConfig | { enabled: TPublicEnabled }; /** Opts into the draft/published lifecycle. Omit to stay on Stage 1 behaviour. */ publication?: ContentPublicationConfig | { enabled: TPublication }; + /** + * Opts into automatic search synchronization. Needs `publication` and + * `publicApi`, and every indexed field must be in `publicApi.fields`. Omit it + * and nothing is indexed. + */ + search?: TSearch; tableName: string; }): ContentTypeDefinition< TId, TFields, TPublication, TPublicField, - TPublicEnabled + TPublicEnabled, + ContentSearchEnabled > => { if (!CONTENT_ID_PATTERN.test(id)) { throw new ContentEngineError( @@ -675,6 +933,16 @@ export const defineContentType = < publicationEnabled, ); + const resolvedSearch = resolveSearch( + id, + fieldMap, + // Same shape of widening as `publicApi` above: the `{ enabled: false }` arm + // exists only so an explicit literal typechecks. + search as ContentSearchConfig | undefined, + resolvedPublicApi, + publicationEnabled, + ); + return { admin: resolvedAdmin, fields, @@ -694,7 +962,8 @@ export const defineContentType = < TFields, TPublication, TPublicField, - TPublicEnabled + TPublicEnabled, + ContentSearchEnabled > >({ admin: resolvedAdmin, @@ -702,6 +971,9 @@ export const defineContentType = < publicApi: resolvedPublicApi, publication: publicationEnabled, }), + search: resolvedSearch as ResolvedContentSearchConfig< + ContentSearchEnabled + >, tableName, }; }; diff --git a/packages/vitnode/src/content/index.ts b/packages/vitnode/src/content/index.ts index 4a88ad4b0..679aae900 100644 --- a/packages/vitnode/src/content/index.ts +++ b/packages/vitnode/src/content/index.ts @@ -52,6 +52,12 @@ export { CONTENT_PUBLICATION_FIELDS, CONTENT_PUBLICATION_STATUS_LENGTH, CONTENT_PUBLICATION_STATUSES, + CONTENT_SEARCH_DESCRIPTION_KINDS, + CONTENT_SEARCH_ITEM_TYPE_MAX_LENGTH, + CONTENT_SEARCH_PATH_MAX_LENGTH, + CONTENT_SEARCH_SLUG_PLACEHOLDER, + CONTENT_SEARCH_TEXT_KINDS, + CONTENT_SEARCH_TITLE_KINDS, CONTENT_SLUG_DEFAULT_LENGTH, CONTENT_SYSTEM_FIELDS, CONTENT_TEXT_DEFAULT_LENGTH, @@ -87,6 +93,11 @@ export { export type { RegisteredContentType } from "./registry"; export { buildContentSchemas } from "./schemas"; export type { ContentSchemas } from "./schemas"; +export { + contentSearchDocumentId, + contentSearchIndexedFieldNames, + contentSearchUrl, +} from "./search"; export { slugify } from "./slug"; export type { AnyContentTypeDefinition, @@ -123,6 +134,10 @@ export type { ContentReferenceField, ContentReferenceFieldName, ContentRelationField, + ContentSearchConfig, + ContentSearchDescriptionField, + ContentSearchTextField, + ContentSearchTitleField, ContentSelect, ContentSlugField, ContentSlugRequired, @@ -138,4 +153,6 @@ export type { ResolvedContentIndex, ResolvedContentPublicApiConfig, ResolvedContentPublicationConfig, + ResolvedContentSearchConfig, + SearchableContentTypeDefinition, } from "./types"; diff --git a/packages/vitnode/src/content/search.test-d.ts b/packages/vitnode/src/content/search.test-d.ts new file mode 100644 index 000000000..97214b36f --- /dev/null +++ b/packages/vitnode/src/content/search.test-d.ts @@ -0,0 +1,333 @@ +import { assertType, describe, expectTypeOf, it } from "vitest"; + +import { + testArticleContentType, + testCategoryContentType, + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import type { + AnyContentTypeDefinition, + SearchableContentTypeDefinition, +} from "./types"; + +import { defineContentType } from "./define"; +import { field } from "./fields"; + +/** The fields every case below reuses. `code` and `author` are never exposed. */ +const fields = { + author: field.user(), + body: field.textarea({ nullable: true }), + code: field.text({ required: true }), + excerpt: field.textarea({ nullable: true }), + featured: field.boolean({ defaultValue: false }), + slug: field.slug({ source: "title" }), + // Public and textual, but nullable - so it is a legal description and an + // illegal title. + subtitle: field.text({ nullable: true }), + title: field.text({ required: true }), + views: field.number({ integer: true, defaultValue: 0 }), +}; + +const publicApi = { + enabled: true, + fields: [ + "title", + "slug", + "excerpt", + "body", + "featured", + "subtitle", + "publishedAt", + ], + path: "articles", +} as const; + +const admin = { + label: { plural: "Articles", singular: "Article" }, + titleField: "title", +} as const; + +describe("search configuration types", () => { + it("accepts a valid configuration", () => { + const definition = defineContentType({ + admin, + fields, + id: "test.valid", + publicApi, + publication: { enabled: true }, + search: { + contentFields: ["excerpt", "body"], + descriptionField: "excerpt", + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }, + tableName: "test_valid", + }); + + expectTypeOf(definition.search.contentFields).toEqualTypeOf(); + expectTypeOf(definition.search.descriptionField).toEqualTypeOf< + null | string + >(); + // The literal survives, which is what makes the definition assignable to + // `SearchableContentTypeDefinition` without an assertion. + expectTypeOf(definition.search.enabled).toEqualTypeOf(); + expectTypeOf(definition).toExtend(); + + const searchable: SearchableContentTypeDefinition = definition; + expectTypeOf(searchable.search.enabled).toEqualTypeOf(); + }); + + it("accepts an explicit `enabled: false`", () => { + const definition = defineContentType({ + admin, + fields, + id: "test.off", + publicApi, + publication: { enabled: true }, + search: { enabled: false }, + tableName: "test_off", + }); + + expectTypeOf(definition.search.enabled).toEqualTypeOf(); + expectTypeOf(definition).not.toExtend(); + }); + + it("resolves an omitted `search` to a literal `false`", () => { + const definition = defineContentType({ + admin, + fields, + id: "test.absent", + publicApi, + publication: { enabled: true }, + tableName: "test_absent", + }); + + expectTypeOf(definition.search.enabled).toEqualTypeOf(); + expectTypeOf(definition).not.toExtend(); + }); + + it("rejects a nullable titleField", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.nullable.title", + publicApi, + publication: { enabled: true }, + search: { + contentFields: ["excerpt"], + enabled: true, + pathTemplate: "/articles/{slug}", + // @ts-expect-error - a nullable field can never be a result heading. + titleField: "subtitle", + }, + tableName: "test_nullable_title", + }), + ); + }); + + it("rejects a widened `enabled`", () => { + const enabled = true as boolean; + + assertType( + defineContentType({ + admin, + fields, + id: "test.widened", + publicApi, + publication: { enabled: true }, + // @ts-expect-error - `enabled` must stay a literal, or every conditional + // in the engine silently resolves to the disabled branch. + search: { enabled }, + tableName: "test_widened", + }), + ); + }); + + it("rejects a titleField that is not a text field", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.title.kind", + publicApi, + publication: { enabled: true }, + search: { + contentFields: ["excerpt"], + enabled: true, + pathTemplate: "/articles/{slug}", + // @ts-expect-error - `views` is a number field. + titleField: "views", + }, + tableName: "test_title_kind", + }), + ); + }); + + it("rejects a textarea titleField", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.title.textarea", + publicApi, + publication: { enabled: true }, + search: { + contentFields: ["excerpt"], + enabled: true, + pathTemplate: "/articles/{slug}", + // @ts-expect-error - prose does not belong in the title slot. + titleField: "excerpt", + }, + tableName: "test_title_textarea", + }), + ); + }); + + it("rejects a descriptionField that is not textual", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.desc.kind", + publicApi, + publication: { enabled: true }, + search: { + contentFields: ["excerpt"], + // @ts-expect-error - `featured` is a boolean field. + descriptionField: "featured", + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }, + tableName: "test_desc_kind", + }), + ); + }); + + it("rejects a private field in contentFields", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.private", + publicApi, + publication: { enabled: true }, + search: { + // @ts-expect-error - `code` is a text field, but it is not in + // `publicApi.fields`, so indexing it would leak it. + contentFields: ["code"], + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }, + tableName: "test_private", + }), + ); + }); + + it("rejects a user field", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.user", + publicApi, + publication: { enabled: true }, + search: { + // @ts-expect-error - a user field can never be public, so it can never + // be indexed either. + contentFields: ["author"], + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }, + tableName: "test_user", + }), + ); + }); + + it("rejects search without a public API", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.nopublic", + publication: { enabled: true }, + search: { + // @ts-expect-error - with no `publicApi` there is no allowlist, so no + // field name is indexable. + contentFields: ["excerpt"], + enabled: true, + pathTemplate: "/articles/{slug}", + // @ts-expect-error - same reason. + titleField: "title", + }, + tableName: "test_nopublic", + }), + ); + }); + + it("rejects an empty contentFields", () => { + assertType( + defineContentType({ + admin, + fields, + id: "test.empty", + publicApi, + publication: { enabled: true }, + search: { + // @ts-expect-error - the tuple type requires at least one entry. + contentFields: [], + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }, + tableName: "test_empty", + }), + ); + }); +}); + +describe("search backward compatibility", () => { + it("keeps every existing fixture assignable to the erased definition", () => { + expectTypeOf(testCategoryContentType).toExtend(); + expectTypeOf(testArticleContentType).toExtend(); + expectTypeOf(testPostContentType).toExtend(); + expectTypeOf( + testSearchablePostContentType, + ).toExtend(); + }); + + it("gives every definition a resolved `search`, enabled or not", () => { + // Stage 1 and Stage 2 fixtures declare no `search` at all, and resolve to a + // literal `false` rather than a widened `boolean`. + expectTypeOf(testCategoryContentType.search.enabled).toEqualTypeOf(); + expectTypeOf(testArticleContentType.search.enabled).toEqualTypeOf(); + expectTypeOf(testPostContentType.search.enabled).toEqualTypeOf(); + expectTypeOf(testPostContentType.search.titleField).toEqualTypeOf(); + }); + + it("satisfies SearchableContentTypeDefinition with no assertion", () => { + // The whole point of the literal: no `as`, anywhere. + const searchable: SearchableContentTypeDefinition = + testSearchablePostContentType; + + expectTypeOf(searchable.search.enabled).toEqualTypeOf(); + expectTypeOf( + testSearchablePostContentType, + ).toExtend(); + }); + + it("keeps a search-less definition out of SearchableContentTypeDefinition", () => { + expectTypeOf( + testPostContentType, + ).not.toExtend(); + expectTypeOf( + testCategoryContentType, + ).not.toExtend(); + }); +}); diff --git a/packages/vitnode/src/content/search.test.ts b/packages/vitnode/src/content/search.test.ts new file mode 100644 index 000000000..47bea6c9e --- /dev/null +++ b/packages/vitnode/src/content/search.test.ts @@ -0,0 +1,302 @@ +import { describe, expect, it } from "vitest"; + +import { + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { defineContentType } from "./define"; +import { field } from "./fields"; +import { + contentSearchDocumentId, + contentSearchIndexedFieldNames, + contentSearchUrl, +} from "./search"; + +const fields = { + author: field.user(), + body: field.textarea({ nullable: true }), + code: field.text({ required: true }), + excerpt: field.textarea({ nullable: true }), + featured: field.boolean({ defaultValue: false }), + slug: field.slug({ source: "title" }), + title: field.text({ required: true }), + views: field.number({ integer: true, defaultValue: 0 }), +}; + +const admin = { + label: { plural: "Articles", singular: "Article" }, + titleField: "title", +} as const; + +const publicApi = { + enabled: true, + fields: ["title", "slug", "excerpt", "body", "featured", "publishedAt"], + path: "articles", +} as const; + +const validSearch = { + contentFields: ["excerpt", "body"], + descriptionField: "excerpt", + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", +} as const; + +/** + * Runtime validation has to hold for a JavaScript caller and for a widened + * TypeScript value, so every case goes in as an untyped `search` object - the + * types are asserted separately in `search.test-d.ts`. + */ +const define = ({ + publication = true, + search, + withPublicApi = true, +}: { + publication?: boolean; + search?: unknown; + withPublicApi?: boolean; +} = {}) => + defineContentType({ + admin, + fields, + id: "test.article", + ...(withPublicApi ? { publicApi } : {}), + ...(publication ? { publication: { enabled: true as const } } : {}), + // `never` because the point of every case below is a value the types reject: + // a widened TypeScript value or a plain-JavaScript caller. The compile-time + // rules are asserted in `search.test-d.ts`. + search: search as never, + tableName: "test_articles_search", + }); + +describe("search configuration", () => { + it("resolves a valid configuration", () => { + const definition = define({ search: validSearch }); + + expect(definition.search).toEqual({ + contentFields: ["excerpt", "body"], + descriptionField: "excerpt", + enabled: true, + pathTemplate: "/articles/{slug}", + titleField: "title", + }); + }); + + it("defaults to disabled when `search` is omitted", () => { + expect(define().search).toEqual({ + contentFields: [], + descriptionField: null, + enabled: false, + pathTemplate: "", + titleField: "", + }); + }); + + it("stays disabled for an explicit `enabled: false`", () => { + expect(define({ search: { enabled: false } }).search.enabled).toBe(false); + }); + + it("leaves every Stage 2 content type untouched", () => { + expect(testPostContentType.search.enabled).toBe(false); + expect(testPostContentType.search.contentFields).toEqual([]); + }); + + describe("required companions", () => { + it("rejects search without publication", () => { + expect(() => + define({ + publication: false, + search: validSearch, + withPublicApi: false, + }), + ).toThrow(/search needs `publication/); + }); + + it("rejects search without a public API", () => { + expect(() => + define({ search: validSearch, withPublicApi: false }), + ).toThrow(/search needs `publicApi/); + }); + }); + + describe("field names", () => { + it("rejects an unknown titleField", () => { + expect(() => + define({ search: { ...validSearch, titleField: "nope" } }), + ).toThrow(/search.titleField references unknown field "nope"/); + }); + + it("rejects a titleField of the wrong kind", () => { + expect(() => + define({ search: { ...validSearch, titleField: "views" } }), + ).toThrow( + /search.titleField names "views" of kind "number"\. Expected one of: text\./, + ); + }); + + it("rejects a private titleField", () => { + expect(() => + define({ search: { ...validSearch, titleField: "code" } }), + ).toThrow(/names "code", which is not in publicApi.fields/); + }); + + it("rejects a descriptionField of the wrong kind", () => { + expect(() => + define({ search: { ...validSearch, descriptionField: "featured" } }), + ).toThrow(/search.descriptionField names "featured" of kind "boolean"/); + }); + + it("accepts an omitted descriptionField", () => { + const { descriptionField } = define({ + search: { ...validSearch, descriptionField: undefined }, + }).search; + + expect(descriptionField).toBeNull(); + }); + + it("rejects a private field in contentFields", () => { + expect(() => + define({ + search: { ...validSearch, contentFields: ["title", "code"] }, + }), + ).toThrow( + /search.contentFields names "code", which is not in publicApi.fields/, + ); + }); + + it("rejects a user field", () => { + expect(() => + define({ search: { ...validSearch, contentFields: ["author"] } }), + ).toThrow(/search.contentFields names "author" of kind "user"/); + }); + + it("rejects a non-textual field in contentFields", () => { + expect(() => + define({ search: { ...validSearch, contentFields: ["featured"] } }), + ).toThrow( + /search.contentFields names "featured" of kind "boolean"\. Expected one of: slug, text, textarea\./, + ); + }); + + it("rejects empty contentFields", () => { + expect(() => + define({ search: { ...validSearch, contentFields: [] } }), + ).toThrow(/search.contentFields is empty/); + }); + + it("rejects duplicate contentFields", () => { + expect(() => + define({ + search: { ...validSearch, contentFields: ["body", "body"] }, + }), + ).toThrow(/search.contentFields lists "body" twice/); + }); + + it("accepts a slug in contentFields", () => { + expect( + define({ search: { ...validSearch, contentFields: ["slug"] } }).search + .contentFields, + ).toEqual(["slug"]); + }); + }); + + describe("pathTemplate", () => { + const withTemplate = (pathTemplate: string) => + define({ search: { ...validSearch, pathTemplate } }); + + it("rejects a template that does not start with a slash", () => { + expect(() => withTemplate("articles/{slug}")).toThrow( + /must start with "\/"/, + ); + }); + + it("rejects a missing placeholder", () => { + expect(() => withTemplate("/articles")).toThrow( + /must contain exactly one "\{slug\}" placeholder, not 0/, + ); + }); + + it("rejects a repeated placeholder", () => { + expect(() => withTemplate("/articles/{slug}/{slug}")).toThrow( + /placeholder, not 2/, + ); + }); + + it("rejects an unknown placeholder", () => { + expect(() => withTemplate("/articles/{id}/{slug}")).toThrow( + /uses a placeholder other than "\{slug\}"/, + ); + }); + + it("rejects traversal, empty segments and whitespace", () => { + expect(() => withTemplate("/articles/../{slug}")).toThrow( + /must not contain an empty segment/, + ); + expect(() => withTemplate("//articles/{slug}")).toThrow( + /must not contain an empty segment/, + ); + expect(() => withTemplate("/articles /{slug}")).toThrow( + /must not contain an empty segment/, + ); + }); + + it("rejects a template longer than the limit", () => { + expect(() => withTemplate(`/${"a".repeat(512)}/{slug}`)).toThrow( + /is longer than 512 characters/, + ); + }); + }); + + it("rejects a content type id wider than the search index column", () => { + expect(() => + defineContentType({ + admin, + fields, + id: `test.${"a".repeat(100)}`, + publicApi, + publication: { enabled: true }, + search: validSearch, + tableName: "test_long_id", + }), + ).toThrow(/is longer than 100 characters/); + }); +}); + +describe("search helpers", () => { + it("builds a relative URL from the template", () => { + expect(contentSearchUrl(testSearchablePostContentType, "hello-world")).toBe( + "/searchable/hello-world", + ); + }); + + it("percent-encodes the slug", () => { + expect(contentSearchUrl(testSearchablePostContentType, "a b/c")).toBe( + "/searchable/a%20b%2Fc", + ); + }); + + it("returns null for an empty slug", () => { + expect(contentSearchUrl(testSearchablePostContentType, " ")).toBeNull(); + }); + + it("returns null when search is off", () => { + expect(contentSearchUrl(testPostContentType, "hello")).toBeNull(); + }); + + it("namespaces the document id by content type", () => { + expect(contentSearchDocumentId(testSearchablePostContentType, 7)).toBe( + "test.searchable:7", + ); + }); + + it("lists every field the document is built from, including the slug", () => { + expect( + contentSearchIndexedFieldNames(testSearchablePostContentType).sort(), + ).toEqual(["body", "excerpt", "slug", "title"]); + }); + + it("lists nothing when search is off", () => { + expect(contentSearchIndexedFieldNames(testPostContentType)).toEqual([]); + }); +}); diff --git a/packages/vitnode/src/content/search.ts b/packages/vitnode/src/content/search.ts new file mode 100644 index 000000000..3978e093c --- /dev/null +++ b/packages/vitnode/src/content/search.ts @@ -0,0 +1,67 @@ +import type { AnyContentTypeDefinition } from "./types"; + +import { CONTENT_SEARCH_SLUG_PLACEHOLDER } from "./const"; + +/** + * The public URL of one record, for a search hit. + * + * `null` for an empty slug rather than a throw: a slug column is `NOT NULL` and + * `slugify` rejects a value that folds to nothing, so this is unreachable + * through the engine - but a row written straight into the database must not + * produce a link to `/articles/`. + * + * `encodeURIComponent` is defence in depth for the same reason. Substitution is + * a single literal replace, and `defineContentType` has already proven the + * template holds exactly one `{slug}` and no other placeholder. + */ +export const contentSearchUrl = ( + definition: AnyContentTypeDefinition, + slug: string, +): null | string => { + const trimmed = slug.trim(); + if (trimmed === "" || definition.search.pathTemplate === "") return null; + + return definition.search.pathTemplate.replace( + CONTENT_SEARCH_SLUG_PLACEHOLDER, + encodeURIComponent(trimmed), + ); +}; + +/** + * A human-readable identifier for one search document, for log lines and the + * AdminCP. + * + * **Not a storage key.** The search index identifies a document by + * `(itemType, itemId, languageCode)`, and the content type id already carries + * the plugin namespace (`plugin.entity`), so there is nothing for a second + * identifier format to disambiguate. + */ +export const contentSearchDocumentId = ( + definition: AnyContentTypeDefinition, + id: number, +): string => `${definition.id}:${id}`; + +/** + * Every field whose value the search document is built from, including the slug + * the URL is built from. + * + * Used to decide whether an update changed anything the index would notice, and + * to project only these columns during a rebuild. + */ +export const contentSearchIndexedFieldNames = ( + definition: AnyContentTypeDefinition, +): string[] => { + const { publicApi, search } = definition; + if (!search.enabled) return []; + + return [ + ...new Set( + [ + search.titleField, + search.descriptionField, + ...search.contentFields, + publicApi.slugField, + ].filter((name): name is string => Boolean(name)), + ), + ]; +}; diff --git a/packages/vitnode/src/content/server/index.ts b/packages/vitnode/src/content/server/index.ts index 9cdad1903..5af1d8153 100644 --- a/packages/vitnode/src/content/server/index.ts +++ b/packages/vitnode/src/content/server/index.ts @@ -41,6 +41,15 @@ export { export { LABEL_PREFIX, resolveReferenceTargets, toLabel } from "./references"; export type { ReferenceTarget } from "./references"; export { buildContentRoutes } from "./routes"; +export { contentSearchDocument } from "./search-document"; +export { createContentSearchIndexer } from "./search-indexer"; +export type { ContentSearchIndexer } from "./search-indexer"; +export { syncContentSearch } from "./search-sync"; +export type { + ContentSearchOperation, + ContentSearchSyncInput, + ContentSearchSyncOutcome, +} from "./search-sync"; export { createContentService } from "./service"; export type { ContentDatabase, diff --git a/packages/vitnode/src/content/server/module.test.ts b/packages/vitnode/src/content/server/module.test.ts new file mode 100644 index 000000000..97cfefb34 --- /dev/null +++ b/packages/vitnode/src/content/server/module.test.ts @@ -0,0 +1,107 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; + +import { + testCategoryContentType, + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { buildApiPlugin } from "../../api/lib/plugin"; +import { createContentModel } from "./model"; +import { buildContentAdminModule } from "./module"; + +const PLUGIN_ID = "@vitnode/example"; + +const categories = createContentModel(testCategoryContentType); +const posts = createContentModel(testPostContentType, { + references: { category: () => categories.table.id }, +}); +const searchablePosts = createContentModel(testSearchablePostContentType); + +const adminModule = ( + contentTypes: Parameters[0]["contentTypes"], +) => buildContentAdminModule({ contentTypes, pluginId: PLUGIN_ID }); + +describe("buildContentAdminModule search indexers", () => { + it("registers an indexer for every searchable content type", () => { + const module = adminModule([categories, posts, searchablePosts]); + + expect(module.searchIndexers?.map(item => item.itemType)).toEqual([ + "test.searchable", + ]); + }); + + it("registers nothing when no content type opted in", () => { + expect(adminModule([categories, posts]).searchIndexers).toEqual([]); + }); + + it("stamps the owning plugin on every generated indexer", () => { + // The indexer keeps its owner because the rebuild runs in the core cron + // request, where `c.get("plugin")` is core rather than the content's plugin. + const [generated] = adminModule([searchablePosts]).searchIndexers ?? []; + + expect(generated).toBeDefined(); + + const plugin = buildApiPlugin({ + pluginId: PLUGIN_ID, + modules: [adminModule([searchablePosts])], + }); + + expect(plugin.searchIndexers?.map(item => item.itemType)).toEqual([ + "test.searchable", + ]); + }); + + it("keeps a manual indexer alongside a generated one", () => { + const plugin = buildApiPlugin({ + pluginId: PLUGIN_ID, + modules: [adminModule([categories, posts, searchablePosts])], + searchIndexers: [ + { + itemType: "example_legacy", + load: async () => + await Promise.resolve({ documents: [], itemsRead: 0 }), + }, + ], + }); + + expect(plugin.searchIndexers?.map(item => item.itemType)).toEqual([ + "example_legacy", + "test.searchable", + ]); + }); + + it("rejects a manual indexer that collides with a generated one", () => { + expect(() => + buildApiPlugin({ + pluginId: PLUGIN_ID, + modules: [adminModule([searchablePosts])], + searchIndexers: [ + { + itemType: "test.searchable", + load: async () => + await Promise.resolve({ documents: [], itemsRead: 0 }), + }, + ], + }), + ).toThrow(/Duplicate search indexer for item type "test.searchable"/); + }); + + it("reaches the plugin through the nested module tree", () => { + const plugin = buildApiPlugin({ + pluginId: PLUGIN_ID, + modules: [adminModule([categories, posts, searchablePosts])], + }); + + expect(plugin.searchIndexers?.map(item => item.itemType)).toEqual([ + "test.searchable", + ]); + // The same module still drives the registry and the permissions. + expect(plugin.contentTypes?.map(item => item.id)).toEqual([ + "test.category", + "test.post", + "test.searchable", + ]); + }); +}); diff --git a/packages/vitnode/src/content/server/module.ts b/packages/vitnode/src/content/server/module.ts index bb201a09e..af9eb837f 100644 --- a/packages/vitnode/src/content/server/module.ts +++ b/packages/vitnode/src/content/server/module.ts @@ -4,6 +4,7 @@ import type { ContentModel } from "./model"; import { buildModule } from "../../api/lib/module"; import { buildContentRoutes } from "./routes"; +import { createContentSearchIndexer } from "./search-indexer"; import { assertContentReferences } from "./table"; /** @@ -24,7 +25,9 @@ import { assertContentReferences } from "./table"; * * That yields `/api/{pluginId}/admin/content/{module}`. `buildApiPlugin` walks * the module tree, so the content types registered here also drive the - * registry and the derived staff permissions - they are declared exactly once. + * registry, the derived staff permissions and - for a content type with + * `search: { enabled: true }` - the generated search indexer. They are declared + * exactly once. */ export const buildContentAdminModule =

({ contentTypes, @@ -52,5 +55,12 @@ export const buildContentAdminModule =

({ routes: [], modules, contentTypes: contentTypes.map(model => model.definition), + // A content type without `search` contributes nothing, so the two module + // builders can keep taking the same array. The plugin id travels with the + // indexer so a rebuild - which runs in the core cron request - still stores + // the owning plugin on every document. + searchIndexers: contentTypes + .filter(model => model.definition.search.enabled) + .map(model => createContentSearchIndexer(model, { pluginId })), }); }; diff --git a/packages/vitnode/src/content/server/routes.ts b/packages/vitnode/src/content/server/routes.ts index 00304fd09..6926f561d 100644 --- a/packages/vitnode/src/content/server/routes.ts +++ b/packages/vitnode/src/content/server/routes.ts @@ -20,6 +20,7 @@ import { orderableColumns } from "../registry"; import { emitContentEvent } from "./emit"; import { withHttpErrors } from "./http-errors"; import { publicationMethods } from "./publication"; +import { syncContentSearch } from "./search-sync"; const zodLabels = z.record(z.string(), z.string().nullable()); @@ -242,6 +243,15 @@ export const buildContentRoutes = < // Emitted only once the write has returned, never inside a transaction. await emitContentEvent(c, definition, "created", { contentId: row.id }); + // A new record is a draft, so this normally indexes nothing - but it is + // computed from the row rather than assumed, the same way the Server + // Action computes its cache tags. + await syncContentSearch(c, definition, { + operation: "create", + pluginId, + row, + }); + return c.json(row, 201); }, }); @@ -280,6 +290,15 @@ export const buildContentRoutes = < }); } + // A slug change is just a rewritten `url`: the search document is keyed by + // item type and id, so there is no stale document to clean up. + await syncContentSearch(c, definition, { + changedFields: result.changedFields, + operation: "update", + pluginId, + row: result.row, + }); + return c.json(result.row, 200); }, }); @@ -329,6 +348,13 @@ export const buildContentRoutes = < ); } + await syncContentSearch(c, definition, { + changed: result.changed, + operation: action, + pluginId, + row: result.row, + }); + return c.json({ changed: result.changed, row: result.row }, 200); }, }); @@ -359,6 +385,15 @@ export const buildContentRoutes = < await emitContentEvent(c, definition, "deleted", { contentId: row.id }); + // `publishedAt` survives an unpublish, so a record that was ever published + // is removed from the index defensively - a delete of a document that is + // not there costs one statement and repairs any drift. + await syncContentSearch(c, definition, { + operation: "delete", + pluginId, + row, + }); + return c.json(row, 200); }, }); diff --git a/packages/vitnode/src/content/server/search-document.test.ts b/packages/vitnode/src/content/server/search-document.test.ts new file mode 100644 index 000000000..973c83696 --- /dev/null +++ b/packages/vitnode/src/content/server/search-document.test.ts @@ -0,0 +1,162 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; + +import { + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { contentSearchDocument } from "./search-document"; + +const PUBLISHED_AT = new Date("2026-02-01T10:00:00.000Z"); +const CREATED_AT = new Date("2026-01-01T00:00:00.000Z"); +const UPDATED_AT = new Date("2026-03-01T00:00:00.000Z"); + +const row = { + author: 3, + body: "The body of the post.", + code: "SECRET-CODE", + createdAt: CREATED_AT, + excerpt: "A short excerpt.", + id: 7, + publishedAt: PUBLISHED_AT, + slug: "hello-world", + status: "published", + title: "Hello world", + updatedAt: UPDATED_AT, + views: 42, +}; + +const document = (overrides: Record = {}) => + contentSearchDocument(testSearchablePostContentType, { + ...row, + ...overrides, + }); + +describe("content search document", () => { + it("maps a published row", () => { + expect(document()).toEqual({ + content: "A short excerpt.\n\nThe body of the post.", + createdAt: PUBLISHED_AT, + isPublic: true, + itemId: 7, + itemType: "test.searchable", + title: "Hello world", + updatedAt: UPDATED_AT, + url: "/searchable/hello-world", + }); + }); + + it("uses the content type id as the item type", () => { + expect(document()?.itemType).toBe(testSearchablePostContentType.id); + }); + + it("leaves the language code unset so every locale matches", () => { + expect(document()).not.toHaveProperty("languageCode"); + }); + + it("omits the owner unless one was given", () => { + expect(document()).not.toHaveProperty("pluginId"); + }); + + it("carries the owning plugin when one is given", () => { + expect( + contentSearchDocument(testSearchablePostContentType, row, { + pluginId: "@vitnode/example", + })?.pluginId, + ).toBe("@vitnode/example"); + }); + + it("never carries an author, container or metadata", () => { + const result = document(); + + expect(result?.authorId).toBeUndefined(); + expect(result?.containerId).toBeUndefined(); + expect(result?.containerType).toBeUndefined(); + expect(result?.metadata).toBeUndefined(); + }); + + it("prefers publishedAt over createdAt", () => { + expect(document()?.createdAt).toEqual(PUBLISHED_AT); + expect( + contentSearchDocument(testSearchablePostContentType, { + ...row, + publishedAt: "2026-02-01T10:00:00.000Z", + })?.createdAt, + ).toEqual(PUBLISHED_AT); + }); + + describe("security", () => { + it("excludes every private field value", () => { + const serialized = JSON.stringify(document()); + + expect(serialized).not.toContain("SECRET-CODE"); + expect(serialized).not.toContain("42"); + expect(serialized).not.toContain('"author"'); + }); + + it("returns null for a draft", () => { + expect(document({ publishedAt: null, status: "draft" })).toBeNull(); + }); + + it("returns null when publishedAt is missing", () => { + expect(document({ publishedAt: null })).toBeNull(); + }); + + it("returns null when publishedAt is in the future", () => { + expect( + document({ publishedAt: new Date(Date.now() + 60_000) }), + ).toBeNull(); + }); + + it("returns null when search is disabled", () => { + expect(contentSearchDocument(testPostContentType, row)).toBeNull(); + }); + }); + + describe("degenerate values", () => { + it("returns null for a blank title", () => { + expect(document({ title: " " })).toBeNull(); + expect(document({ title: null })).toBeNull(); + }); + + it("returns null for a blank slug", () => { + expect(document({ slug: "" })).toBeNull(); + }); + + it("returns null for a non-numeric id", () => { + expect(document({ id: "7" })).toBeNull(); + }); + + it("keeps a document with no body at all", () => { + const result = document({ body: null, excerpt: null }); + + expect(result?.content).toBe(""); + expect(result?.title).toBe("Hello world"); + }); + }); + + describe("text handling", () => { + it("collapses whitespace in the title", () => { + expect(document({ title: " Hello \n\t world " })?.title).toBe( + "Hello world", + ); + }); + + it("concatenates content fields in order and skips the empty ones", () => { + expect(document({ excerpt: null })?.content).toBe( + "The body of the post.", + ); + expect(document({ body: null })?.content).toBe("A short excerpt."); + }); + + it("does not index the description twice", () => { + // `excerpt` is both the description and the first content field. + expect(document()?.content.split("A short excerpt.")).toHaveLength(2); + }); + + it("percent-encodes an unusual slug", () => { + expect(document({ slug: "a b" })?.url).toBe("/searchable/a%20b"); + }); + }); +}); diff --git a/packages/vitnode/src/content/server/search-document.ts b/packages/vitnode/src/content/server/search-document.ts new file mode 100644 index 000000000..a3c77fc36 --- /dev/null +++ b/packages/vitnode/src/content/server/search-document.ts @@ -0,0 +1,131 @@ +import type { SearchDocument } from "../../api/models/search"; +import type { AnyContentTypeDefinition } from "../types"; + +import { isContentPubliclyVisible } from "../cache"; +import { contentSearchUrl } from "../search"; + +/** Collapses whitespace so a multi-line value cannot break a result heading. */ +const normalize = (value: unknown): string => + typeof value === "string" ? value.replace(/\s+/g, " ").trim() : ""; + +const toDate = (value: unknown): Date | undefined => { + if (value instanceof Date) return value; + if (typeof value !== "string") return undefined; + + const parsed = new Date(value); + + return Number.isNaN(parsed.getTime()) ? undefined : parsed; +}; + +const toTimestamp = (value: unknown): Date | null | string | undefined => { + if (value instanceof Date || typeof value === "string") return value; + + return value === null ? null : undefined; +}; + +/** + * Whether one row is currently publicly visible. + * + * {@link isContentPubliclyVisible} with the column coercion in front of it, so + * "is this public" and "can this be indexed" stay two separate questions - a + * published record whose title is blank is the first but not the second, and it + * needs its stale document removed rather than left alone. + */ +export const isContentRowPublic = (row: object): boolean => { + const values = row as Record; + + return isContentPubliclyVisible({ + publishedAt: toTimestamp(values.publishedAt), + status: normalize(values.status) || undefined, + }); +}; + +/** + * Projects one content record into a search document. + * + * Returns `null` - not a partial document - whenever the record must not be + * indexed: search is off, the row is not publicly visible, or the title or slug + * is missing. Every caller treats `null` as "make sure nothing is indexed for + * this record", so there is one decision and not one per call site. + * + * Visibility is {@link isContentPubliclyVisible}, the same predicate the Server + * Actions use to decide which cache tags to expire. The engine has exactly two + * definitions of "public" - that one and `publishedCondition` in SQL - and this + * adds no third. + * + * Nothing outside `publicApi.fields` can reach the document: `defineContentType` + * has already proven every indexed field name is in that allowlist, so a private + * column cannot be read here even by mistake. + * + * `row` is `object` rather than `ContentSelect` because columns are + * read by a name resolved at runtime, and an unresolved generic row type is not + * assignable to an index signature - typing it strictly would push a cast to + * every call site instead of keeping the one honest cast here. + */ +export const contentSearchDocument = ( + definition: AnyContentTypeDefinition, + row: object, + { pluginId }: { pluginId?: string } = {}, +): null | SearchDocument => { + const { publicApi, search } = definition; + if (!search.enabled) return null; + + const values = row as Record; + + const itemId = values.id; + if (typeof itemId !== "number") return null; + + if (!isContentRowPublic(row)) return null; + + const title = normalize(values[search.titleField]); + if (title === "") return null; + + const url = contentSearchUrl( + definition, + normalize(values[publicApi.slugField]), + ); + if (url === null) return null; + + // A published row always carries `publishedAt`, so this resolves to the + // publication date - which is the date the feed and the timeline sort by. + const createdAt = toDate(values.publishedAt) ?? toDate(values.createdAt); + if (!createdAt) return null; + + // The description leads the body so it shows up first in a result excerpt. + // Skipped when it is already one of the content fields, so it is not indexed + // twice and does not distort ranking. + const sources = search.contentFields.includes(search.descriptionField ?? "") + ? search.contentFields + : [ + ...(search.descriptionField ? [search.descriptionField] : []), + ...search.contentFields, + ]; + + return { + // `SearchModel.index` strips HTML from `content` (but never from `title`, + // which is why the title is normalized above). + content: sources + .map(name => normalize(values[name])) + .filter(value => value !== "") + .join("\n\n"), + createdAt, + // Only publicly visible rows get this far, and an unpublished record is + // deleted from the index rather than hidden in it. + isPublic: true, + itemId, + // Content type ids are globally unique and already namespaced as + // `plugin.entity`, so the id alone is a collision-free item type. + itemType: definition.id, + // The owning plugin, so a rebuild - which runs in the core cron request - + // stores the same ownership a live write did. + ...(pluginId === undefined ? {} : { pluginId }), + // 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. + title, + updatedAt: toDate(values.updatedAt), + url, + }; +}; diff --git a/packages/vitnode/src/content/server/search-indexer.test.ts b/packages/vitnode/src/content/server/search-indexer.test.ts new file mode 100644 index 000000000..d16bce27b --- /dev/null +++ b/packages/vitnode/src/content/server/search-indexer.test.ts @@ -0,0 +1,232 @@ +// @vitest-environment node +import type { Context } from "hono"; + +import { describe, expect, it } from "vitest"; + +import { + testCategoryContentType, + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { createContentModel } from "./model"; +import { createContentSearchIndexer } from "./search-indexer"; + +interface RecordedCall { + arg: unknown; + op: string; +} + +/** The same chainable Drizzle stand-in `service.test.ts` uses, plus `offset`. */ +const createDbMock = (results: unknown[][]) => { + const calls: RecordedCall[] = []; + const queue = [...results]; + + const chain = (rows: unknown[]) => { + const record = (op: string, arg: unknown) => { + calls.push({ arg, op }); + + return builder; + }; + + const builder = { + from: (value: unknown) => record("from", value), + limit: (value: unknown) => record("limit", value), + offset: (value: unknown) => record("offset", value), + orderBy: (value: unknown) => record("orderBy", value), + then: async (resolve: (rows: unknown[]) => TResult) => + Promise.resolve(rows).then(resolve), + where: (value: unknown) => record("where", value), + }; + + return builder; + }; + + const db = { + select: (arg: unknown) => { + calls.push({ arg, op: "select" }); + + return chain(queue.shift() ?? []); + }, + }; + + const c = { + get: (key: string) => (key === "db" ? db : undefined), + } as Context; + + return { c, calls }; +}; + +const opOf = (calls: RecordedCall[], op: string) => + calls.find(call => call.op === op)?.arg; + +const categories = createContentModel(testCategoryContentType); +const searchable = createContentModel(testSearchablePostContentType); +const plain = createContentModel(testPostContentType, { + references: { category: () => categories.table.id }, +}); + +const PLUGIN_ID = "@vitnode/example"; + +const indexerFor = (model: typeof plain | typeof searchable) => + createContentSearchIndexer(model as typeof searchable, { + pluginId: PLUGIN_ID, + }); + +const PUBLISHED_AT = new Date("2026-02-01T10:00:00.000Z"); + +const dbRow = (id: number, slug: string) => ({ + body: "Body copy.", + code: "SECRET", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + excerpt: "Excerpt.", + id, + publishedAt: PUBLISHED_AT, + slug, + status: "published", + title: `Post ${id}`, + updatedAt: new Date("2026-01-02T00:00:00.000Z"), +}); + +describe("generated content search indexer", () => { + it("uses the content type id as the item type", () => { + expect(indexerFor(searchable).itemType).toBe("test.searchable"); + }); + + describe("count", () => { + it("counts only published rows", async () => { + const { c, calls } = createDbMock([[{ value: 12 }]]); + + const total = await indexerFor(searchable).count?.(c); + + expect(total).toBe(12); + // The published predicate is not optional, so there is always a `where`. + expect(calls.some(call => call.op === "where" && call.arg)).toBe(true); + }); + + it("reports zero for an empty table", async () => { + const { c } = createDbMock([[]]); + + await expect(indexerFor(searchable).count?.(c)).resolves.toBe(0); + }); + }); + + describe("load", () => { + it("projects only the columns the document needs", async () => { + const { c, calls } = createDbMock([[]]); + + await indexerFor(searchable).load(c, 0, 200); + + const selection = opOf(calls, "select") as Record; + + expect(Object.keys(selection).sort()).toEqual([ + "body", + "createdAt", + "excerpt", + "id", + "publishedAt", + "slug", + "status", + "title", + "updatedAt", + ]); + // The private columns are never even fetched. + expect(selection).not.toHaveProperty("code"); + expect(selection).not.toHaveProperty("views"); + expect(selection).not.toHaveProperty("author"); + }); + + it("orders deterministically and honours the page window", async () => { + const { c, calls } = createDbMock([[]]); + + await indexerFor(searchable).load(c, 400, 200); + + expect(opOf(calls, "orderBy")).toBeDefined(); + expect(opOf(calls, "limit")).toBe(200); + expect(opOf(calls, "offset")).toBe(400); + }); + + it("maps every row into a document, and stamps the owning plugin", async () => { + const { c } = createDbMock([[dbRow(1, "one"), dbRow(2, "two")]]); + + const page = await indexerFor(searchable).load(c, 0, 200); + + expect(page.itemsRead).toBe(2); + expect(page.documents).toHaveLength(2); + expect(page.documents[0]).toMatchObject({ + itemId: 1, + itemType: "test.searchable", + pluginId: PLUGIN_ID, + title: "Post 1", + url: "/searchable/one", + }); + expect(page.documents[1]).toMatchObject({ + pluginId: PLUGIN_ID, + url: "/searchable/two", + }); + }); + + it("reports zero items read past the end of the source", async () => { + const { c } = createDbMock([[]]); + + await expect(indexerFor(searchable).load(c, 1000, 200)).resolves.toEqual({ + documents: [], + itemsRead: 0, + }); + }); + + it("counts rows the mapper rejected as items read", async () => { + // The regression this contract exists for: a page can read rows and + // project none of them, and reporting that as "no items" would end the + // rebuild before the valid rows behind it. + const { c } = createDbMock([ + [ + { ...dbRow(1, "one"), title: " " }, + { ...dbRow(2, "two"), title: null }, + ], + ]); + + const page = await indexerFor(searchable).load(c, 0, 200); + + expect(page.itemsRead).toBe(2); + expect(page.documents).toEqual([]); + }); + + it("separates valid from invalid rows on a mixed page", async () => { + const { c, calls } = createDbMock([ + [ + dbRow(1, "one"), + { ...dbRow(2, "two"), title: " " }, + dbRow(3, "three"), + ], + ]); + + const page = await indexerFor(searchable).load(c, 0, 200); + + // `itemsRead` is the row count; only the valid rows became documents. + expect(page.itemsRead).toBe(3); + expect(page.documents.map(document => document.itemId)).toEqual([1, 3]); + + const selection = opOf(calls, "select") as Record; + expect(selection).not.toHaveProperty("code"); + expect(JSON.stringify(page.documents)).not.toContain("SECRET"); + }); + }); + + it("refuses a content type without publication", () => { + expect(() => + indexerFor(categories as unknown as typeof searchable), + ).toThrow(/publication/i); + }); + + it("builds for a content type with search off, but yields no documents", async () => { + const { c } = createDbMock([[dbRow(1, "one")]]); + + // `buildContentAdminModule` filters these out; the mapper is the backstop. + // The row is still read, so `itemsRead` reflects it. + await expect(indexerFor(plain).load(c, 0, 200)).resolves.toEqual({ + documents: [], + itemsRead: 1, + }); + }); +}); diff --git a/packages/vitnode/src/content/server/search-indexer.ts b/packages/vitnode/src/content/server/search-indexer.ts new file mode 100644 index 000000000..3a95739d1 --- /dev/null +++ b/packages/vitnode/src/content/server/search-indexer.ts @@ -0,0 +1,131 @@ +import type { + PgColumn, + PgTableWithColumns, + TableConfig, +} from "drizzle-orm/pg-core"; +import type { Context } from "hono"; + +import { asc, count } from "drizzle-orm"; + +import type { + SearchDocument, + SearchIndexer, + SearchIndexerPage, +} from "../../api/models/search"; +import type { AnyContentTypeDefinition } from "../types"; +import type { ContentModel } from "./model"; + +import { contentSearchIndexedFieldNames } from "../search"; +import { publicationColumns, publishedCondition } from "./publication"; +import { contentSearchDocument } from "./search-document"; + +/** System columns the mapper reads, on top of the configured search fields. */ +const REQUIRED_COLUMNS = [ + "id", + "createdAt", + "updatedAt", + "status", + "publishedAt", +] as const; + +/** + * A generated indexer, pinned to the modern page contract. + * + * `SearchIndexer.load` also accepts the deprecated bare-array result, for + * hand-written indexers that predate it. A generated one never returns that, and + * saying so keeps the guarantee in the type rather than in a comment. + */ +export interface ContentSearchIndexer extends SearchIndexer { + load: ( + c: Context, + offset: number, + limit: number, + ) => Promise; +} + +/** + * Adapts one content type to the engine's {@link SearchIndexer} contract, so a + * full or per-collection rebuild can stream its published records. + * + * Registered automatically by `buildContentAdminModule` for every content type + * with `search: { enabled: true }` - manual indexers registered by a plugin are + * untouched and keep working exactly as before. + * + * Two properties matter for review: + * + * 1. **Only published rows are read.** `publishedCondition` is not a parameter - + * both queries `and` it in themselves, so there is no argument a caller could + * forget, and it is the same SQL predicate the public read layer uses. + * 2. **Only projected columns are read.** The `SELECT` is built from the + * configured search fields, all of which `defineContentType` has already + * proven are in `publicApi.fields`. A private column is never fetched, and + * column names are resolved into Drizzle columns rather than interpolated + * into SQL. + */ +export const createContentSearchIndexer = < + TDefinition extends AnyContentTypeDefinition, +>( + model: ContentModel, + { pluginId }: { pluginId: string }, +): ContentSearchIndexer => { + const { definition } = model; + // Widened the same way `createContentPublicService` takes it: the query + // builders are written against the erased table, not this content type's. + const table: PgTableWithColumns = model.table; + const columns = model.columns as Record; + const published = publicationColumns(definition, columns); + const primaryCursor = columns.id; + + const selection: Record = Object.fromEntries( + [ + ...new Set([ + ...REQUIRED_COLUMNS, + ...contentSearchIndexedFieldNames(definition), + ]), + ].map(name => [name, columns[name]]), + ); + + return { + itemType: definition.id, + + // Published rows, not every row: the AdminCP coverage bar compares this + // against the number of indexed items, and counting drafts would pin a + // mostly-unpublished collection at "stale" forever. + count: async c => { + const [row] = await c + .get("db") + .select({ value: count() }) + .from(table) + .where(publishedCondition(published)); + + return row?.value ?? 0; + }, + + // Offset paging, which is what the contract exposes. Ordering by the primary + // key keeps pages from overlapping within one rebuild; a row whose + // publication state changes mid-rebuild can still shift, and that is what + // the next publish - or the next rebuild - repairs. + // + // `itemsRead` is the row count, not the document count. A published row with + // no usable title projects to nothing, and reporting that as "no items" would + // end the rebuild before the valid rows after it. + load: async (c, offset, limit) => { + const rows = await c + .get("db") + .select(selection) + .from(table) + .where(publishedCondition(published)) + .orderBy(asc(primaryCursor)) + .limit(limit) + .offset(offset); + + const documents = rows.flatMap(row => { + const document = contentSearchDocument(definition, row, { pluginId }); + + return document ? [document] : []; + }) satisfies SearchDocument[]; + + return { documents, itemsRead: rows.length }; + }, + }; +}; diff --git a/packages/vitnode/src/content/server/search-sync.test.ts b/packages/vitnode/src/content/server/search-sync.test.ts new file mode 100644 index 000000000..5ccd529b9 --- /dev/null +++ b/packages/vitnode/src/content/server/search-sync.test.ts @@ -0,0 +1,481 @@ +// @vitest-environment node +import type { Context, MiddlewareHandler } from "hono"; + +import { OpenAPIHono } from "@hono/zod-openapi"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { + testCategoryContentType, + testPostContentType, + testSearchablePostContentType, +} from "@/tests/content-fixtures"; + +import { createContentModel } from "./model"; +import { buildContentRoutes } from "./routes"; + +vi.mock("../../api/lib/check-staff-permission", () => ({ + assertStaffPermission: async () => { + await Promise.resolve(); + }, +})); + +const categories = createContentModel(testCategoryContentType); +const searchable = createContentModel(testSearchablePostContentType); +const plain = createContentModel(testPostContentType, { + references: { category: () => categories.table.id }, +}); + +const PLUGIN_ID = "@vitnode/example"; + +const PUBLISHED_AT = new Date("2026-02-01T10:00:00.000Z"); +const CREATED_AT = new Date("2026-01-01T00:00:00.000Z"); + +const publishedRow = { + author: 3, + body: "Body copy.", + code: "SECRET", + createdAt: CREATED_AT, + excerpt: "Excerpt.", + id: 7, + publishedAt: PUBLISHED_AT, + slug: "hello-world", + status: "published" as const, + title: "Hello world", + updatedAt: CREATED_AT, + views: 0, +}; + +const draftRow = { + ...publishedRow, + publishedAt: null, + status: "draft" as const, +}; + +/** + * The generated routes with the service, the search engine and the logger + * stubbed, so each case drives the real handler and asserts on what reached + * `c.get("search")`. + */ +const harness = ({ + logFails = false, + model = searchable, + searchFails = false, +}: { + logFails?: boolean; + model?: typeof plain | typeof searchable; + searchFails?: boolean; +} = {}) => { + const logged: string[] = []; + const search = { + delete: vi.fn(async () => { + if (searchFails) throw new Error("engine unavailable"); + await Promise.resolve(); + }), + index: vi.fn(async () => { + if (searchFails) throw new Error("engine unavailable"); + await Promise.resolve(); + }), + }; + const service = { + create: vi.fn(), + delete: vi.fn(), + findById: vi.fn(), + findMany: vi.fn(), + options: vi.fn(), + publish: vi.fn(), + unpublish: vi.fn(), + update: vi.fn(), + }; + + vi.spyOn(model, "service").mockReturnValue(service); + + const app = new OpenAPIHono(); + const context: MiddlewareHandler = async (c, next) => { + c.set("events", { + emit: async () => { + await Promise.resolve(); + }, + } as unknown as Context["var"]["events"]); + c.set("search", search as unknown as Context["var"]["search"]); + c.set("log", { + debug: async () => { + await Promise.resolve(); + }, + error: async (content: string) => { + await Promise.resolve(); + logged.push(content); + if (logFails) throw new Error("core_logs unavailable"); + }, + warn: async () => { + await Promise.resolve(); + }, + }); + c.set("admin", { user: { id: 1 } } as unknown as Context["var"]["admin"]); + await next(); + }; + app.use("*", context); + + // The two fixtures are structurally different content types, and the harness + // only needs their routes - so the union collapses here rather than making + // every caller generic. + for (const { handler, route } of buildContentRoutes( + model as typeof searchable, + { pluginId: PLUGIN_ID }, + )) { + app.openapi(route, handler); + } + + return { app, logged, search, service }; +}; + +const json = (body: unknown) => ({ + body: JSON.stringify(body), + headers: { "Content-Type": "application/json" }, +}); + +describe("content search lifecycle synchronization", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + describe("create", () => { + it("indexes nothing for a new draft", async () => { + const { app, search, service } = harness(); + service.create.mockResolvedValue(draftRow); + + const res = await app.request("/", { + ...json({ code: "a", slug: "hello-world", title: "Hello world" }), + method: "POST", + }); + + expect(res.status).toBe(201); + expect(search.index).not.toHaveBeenCalled(); + expect(search.delete).not.toHaveBeenCalled(); + }); + }); + + describe("update", () => { + it("indexes nothing for a draft", async () => { + const { app, search, service } = harness(); + service.update.mockResolvedValue({ + changedFields: ["title"], + row: draftRow, + }); + + const res = await app.request("/7", { + ...json({ title: "Changed" }), + method: "PUT", + }); + + expect(res.status).toBe(200); + expect(search.index).not.toHaveBeenCalled(); + }); + + it("upserts when a published record's indexed field changes", async () => { + const { app, search, service } = harness(); + service.update.mockResolvedValue({ + changedFields: ["title"], + row: publishedRow, + }); + + await app.request("/7", { ...json({ title: "Changed" }), method: "PUT" }); + + expect(search.index).toHaveBeenCalledTimes(1); + expect(search.index).toHaveBeenCalledWith( + expect.objectContaining({ + itemId: 7, + itemType: "test.searchable", + title: "Hello world", + }), + ); + }); + + it("indexes nothing when only a non-indexed field changes", async () => { + const { app, search, service } = harness(); + service.update.mockResolvedValue({ + changedFields: ["views"], + row: publishedRow, + }); + + await app.request("/7", { ...json({ views: 1 }), method: "PUT" }); + + expect(search.index).not.toHaveBeenCalled(); + }); + + it("rewrites the url when the slug changes", async () => { + const { app, search, service } = harness(); + service.update.mockResolvedValue({ + changedFields: ["slug"], + row: { ...publishedRow, slug: "renamed" }, + }); + + await app.request("/7", { ...json({ slug: "renamed" }), method: "PUT" }); + + expect(search.index).toHaveBeenCalledWith( + expect.objectContaining({ url: "/searchable/renamed" }), + ); + // No stale document: the key is the item type and id, not the url. + expect(search.delete).not.toHaveBeenCalled(); + }); + + it("removes the document when a published record stops being indexable", async () => { + const { app, search, service } = harness(); + // Still published, but there is no longer a title to show in a result. + service.update.mockResolvedValue({ + changedFields: ["title"], + row: { ...publishedRow, title: " " }, + }); + + await app.request("/7", { ...json({ title: " " }), method: "PUT" }); + + expect(search.delete).toHaveBeenCalledWith("test.searchable", 7); + expect(search.index).not.toHaveBeenCalled(); + }); + + it("indexes nothing when nothing changed", async () => { + const { app, search, service } = harness(); + service.update.mockResolvedValue({ + changedFields: [], + row: publishedRow, + }); + + await app.request("/7", { + ...json({ title: "Hello world" }), + method: "PUT", + }); + + expect(search.index).not.toHaveBeenCalled(); + }); + }); + + describe("publish", () => { + it("upserts once on a real transition", async () => { + const { app, search, service } = harness(); + service.publish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: publishedRow, + }); + + const res = await app.request("/7/publish", { method: "POST" }); + + expect(res.status).toBe(200); + expect(search.index).toHaveBeenCalledTimes(1); + expect(search.index).toHaveBeenCalledWith( + expect.objectContaining({ + content: "Excerpt.\n\nBody copy.", + createdAt: PUBLISHED_AT, + isPublic: true, + // Stamped by the route, so a rebuild reproduces the same ownership. + pluginId: PLUGIN_ID, + url: "/searchable/hello-world", + }), + ); + }); + + it("does nothing when the record was already published", async () => { + const { app, search, service } = harness(); + service.publish.mockResolvedValue({ + changed: false, + publishedAt: PUBLISHED_AT, + row: publishedRow, + }); + + await app.request("/7/publish", { method: "POST" }); + + expect(search.index).not.toHaveBeenCalled(); + expect(search.delete).not.toHaveBeenCalled(); + }); + }); + + describe("unpublish", () => { + it("deletes the document on a real transition", async () => { + const { app, search, service } = harness(); + service.unpublish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: { ...publishedRow, status: "draft" as const }, + }); + + await app.request("/7/unpublish", { method: "POST" }); + + expect(search.delete).toHaveBeenCalledTimes(1); + expect(search.delete).toHaveBeenCalledWith("test.searchable", 7); + expect(search.index).not.toHaveBeenCalled(); + }); + + it("does nothing when the record was already a draft", async () => { + const { app, search, service } = harness(); + service.unpublish.mockResolvedValue({ + changed: false, + publishedAt: null, + row: draftRow, + }); + + await app.request("/7/unpublish", { method: "POST" }); + + expect(search.delete).not.toHaveBeenCalled(); + }); + }); + + describe("delete", () => { + it("deletes the document for a published record", async () => { + const { app, search, service } = harness(); + service.delete.mockResolvedValue(publishedRow); + + await app.request("/7", { method: "DELETE" }); + + expect(search.delete).toHaveBeenCalledWith("test.searchable", 7); + }); + + it("deletes defensively for a record that was published before", async () => { + const { app, search, service } = harness(); + // `publishedAt` survives an unpublish, so this row was indexed once. + service.delete.mockResolvedValue({ + ...publishedRow, + status: "draft" as const, + }); + + await app.request("/7", { method: "DELETE" }); + + expect(search.delete).toHaveBeenCalledWith("test.searchable", 7); + }); + + it("does nothing for a never-published draft", async () => { + const { app, search, service } = harness(); + service.delete.mockResolvedValue(draftRow); + + await app.request("/7", { method: "DELETE" }); + + expect(search.delete).not.toHaveBeenCalled(); + }); + }); + + describe("failure handling", () => { + it("keeps the mutation successful when the engine throws", async () => { + const { app, logged, service } = harness({ searchFails: true }); + service.publish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: publishedRow, + }); + + const res = await app.request("/7/publish", { method: "POST" }); + + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ changed: true }); + expect(logged).toHaveLength(1); + expect(logged[0]).toContain("[content-search]"); + expect(logged[0]).toContain("test.searchable"); + expect(logged[0]).toContain("engine unavailable"); + }); + + it("logs structured context", async () => { + const { app, logged, service } = harness({ searchFails: true }); + service.delete.mockResolvedValue(publishedRow); + + await app.request("/7", { method: "DELETE" }); + + const payload: unknown = JSON.parse( + logged[0].slice(logged[0].indexOf("{")), + ); + + expect(payload).toMatchObject({ + action: "delete", + contentTypeId: "test.searchable", + documentId: "test.searchable:7", + itemId: 7, + itemType: "test.searchable", + operation: "delete", + pluginId: PLUGIN_ID, + }); + }); + + it("writes no error log when synchronization succeeds", async () => { + const { app, logged, search, service } = harness(); + service.publish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: publishedRow, + }); + + const res = await app.request("/7/publish", { method: "POST" }); + + expect(res.status).toBe(200); + expect(search.index).toHaveBeenCalledTimes(1); + expect(logged).toEqual([]); + }); + + it("keeps the mutation successful when the logger fails too", async () => { + // The logger writes to the database, so it can be down for the same reason + // the search engine is. Both are best effort after a committed write. + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + const { app, logged, service } = harness({ + logFails: true, + searchFails: true, + }); + service.publish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: publishedRow, + }); + + const res = await app.request("/7/publish", { method: "POST" }); + + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ changed: true }); + + // The structured line was attempted, then the console stood in for it - + // still carrying the original search error, not the logger's. + expect(logged).toHaveLength(1); + expect(consoleError).toHaveBeenCalledTimes(1); + const fallback = String(consoleError.mock.calls[0][0]); + expect(fallback).toContain("Failed to log content search failure"); + expect(fallback).toContain("engine unavailable"); + expect(fallback).not.toContain("core_logs unavailable"); + } finally { + consoleError.mockRestore(); + } + }); + + it("does not reach the console when only the engine fails", async () => { + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + const { app, service } = harness({ searchFails: true }); + service.delete.mockResolvedValue(publishedRow); + + const res = await app.request("/7", { method: "DELETE" }); + + expect(res.status).toBe(200); + expect(consoleError).not.toHaveBeenCalled(); + } finally { + consoleError.mockRestore(); + } + }); + }); + + describe("content types without search", () => { + it("never touches the search engine", async () => { + const { app, search, service } = harness({ model: plain }); + service.publish.mockResolvedValue({ + changed: true, + publishedAt: PUBLISHED_AT, + row: { ...publishedRow, category: 1 }, + }); + service.delete.mockResolvedValue({ ...publishedRow, category: 1 }); + + await app.request("/7/publish", { method: "POST" }); + await app.request("/7", { method: "DELETE" }); + + expect(search.index).not.toHaveBeenCalled(); + 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 new file mode 100644 index 000000000..5324af904 --- /dev/null +++ b/packages/vitnode/src/content/server/search-sync.ts @@ -0,0 +1,179 @@ +import type { Context } from "hono"; + +import type { AnyContentTypeDefinition } from "../types"; + +import { + contentSearchDocumentId, + contentSearchIndexedFieldNames, +} from "../search"; +import { contentSearchDocument, isContentRowPublic } from "./search-document"; + +/** Which mutation just returned. Not an event name: nothing is emitted here. */ +export type ContentSearchOperation = + "create" | "delete" | "publish" | "unpublish" | "update"; + +export interface ContentSearchSyncInput { + /** + * `publish` / `unpublish` only: `false` when the record was already in the + * requested state, which means the index already agrees and there is nothing + * to do. + */ + changed?: boolean; + /** `update` only. An update that touched no indexed field changes no document. */ + changedFields?: readonly string[]; + operation: ContentSearchOperation; + /** + * The plugin that owns the content type. Stamped on the document so a rebuild + * reproduces the same ownership; omit it and the request's plugin is used, + * which is only correct while the request belongs to the owner. + */ + pluginId?: string; + /** The full row the mutation returned, including `status` and `publishedAt`. */ + row: object; +} + +export interface ContentSearchSyncOutcome { + action: "delete" | "skip" | "upsert"; + /** `"example.article:7"` - for logs and diagnostics, never a storage key. */ + documentId: string; + /** Set when the search engine threw. The mutation itself still succeeded. */ + error?: Error; +} + +/** What the row is, and what the index therefore has to hold. */ +const decide = ( + definition: AnyContentTypeDefinition, + { changed, changedFields, operation, row }: ContentSearchSyncInput, + isPublic: boolean, +): "delete" | "skip" | "upsert" => { + const values = row as Record; + + if (operation === "delete") { + // `publishedAt` survives an unpublish, so this covers both "currently + // published" and "was published, is a draft now" - and skips a record that + // was never published, which was never indexed. + return values.publishedAt === null || values.publishedAt === undefined + ? "skip" + : "delete"; + } + + if (operation === "unpublish") return changed === true ? "delete" : "skip"; + + // An idempotent publish is a no-op for the same reason it emits no event: + // the document is already there and would be rewritten byte for byte. + if (operation === "publish") { + return changed === true && isPublic ? "upsert" : "skip"; + } + + if (operation === "create") return isPublic ? "upsert" : "skip"; + + // `update` cannot change `status`, so a draft stays a draft and a published + // record stays published. Nothing to delete, and nothing to write unless a + // field the document is built from actually moved. + if (!isPublic) return "skip"; + + const indexed = new Set(contentSearchIndexedFieldNames(definition)); + + return (changedFields ?? []).some(name => indexed.has(name)) + ? "upsert" + : "skip"; +}; + +/** + * Brings the search index in line with one content mutation. + * + * **Call it only once the database write has returned - never inside a + * transaction callback.** A rolled-back transaction would leave a document + * pointing at a record that does not exist, and the search index is not part of + * the transaction that could undo it. This is the same rule the Next cache + * invalidation follows, for the same reason. + * + * The generated admin routes call it for you. A direct `service.publish(id)` + * call does not, deliberately: it may be running inside a caller-provided + * transaction. Application code opts in explicitly, after commit: + * + * ```ts + * const result = await model.service(c).publish(id); + * if (result) { + * await syncContentSearch(c, articleContentType, { + * operation: "publish", + * changed: result.changed, + * row: result.row, + * }); + * } + * ``` + * + * A failing search engine never turns a successful write into a failed one. The + * error is logged with enough context to find the record, the outcome carries it + * for a caller that wants it, and a manual rebuild repairs the drift. That makes + * the index eventually consistent, with "eventually" bounded by the next publish + * or the next rebuild. + */ +export const syncContentSearch = async ( + c: Context, + definition: AnyContentTypeDefinition, + input: ContentSearchSyncInput, +): Promise => { + const values = input.row as Record; + const itemId = typeof values.id === "number" ? values.id : 0; + const documentId = contentSearchDocumentId(definition, itemId); + + if (!definition.search.enabled || itemId === 0) { + return { action: "skip", documentId }; + } + + const decided = decide(definition, input, isContentRowPublic(input.row)); + if (decided === "skip") return { action: decided, documentId }; + + // A publicly visible row the mapper will not build - a title that is only + // whitespace, say - has its document removed rather than left holding whatever + // text it was indexed with last time. + const document = + decided === "upsert" + ? contentSearchDocument(definition, input.row, { + pluginId: input.pluginId, + }) + : null; + const action = decided === "upsert" && !document ? "delete" : decided; + + try { + if (document) { + await c.get("search").index(document); + } else { + await c.get("search").delete(definition.id, itemId); + } + + return { action, documentId }; + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + + // `c.get("log")` takes a string, so the context goes in as JSON behind a + // greppable prefix. The logger middleware adds the plugin id, path, method, + // user and timestamp on its way into `core_logs`. + const message = `[content-search] ${JSON.stringify({ + action, + contentTypeId: definition.id, + documentId, + error: error.message, + itemId, + itemType: definition.id, + operation: input.operation, + pluginId: input.pluginId, + })}`; + + // The logger writes to the database, so it can fail for the same reason the + // search engine just did. Both are best effort *after* a committed write, and + // neither may turn it into a failed request - so the fallback is the console, + // and the outcome keeps the original search error rather than this one. + 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/types.ts b/packages/vitnode/src/content/types.ts index 2f2d4c236..dfcc4e4ba 100644 --- a/packages/vitnode/src/content/types.ts +++ b/packages/vitnode/src/content/types.ts @@ -3,6 +3,9 @@ import type { CONTENT_PUBLIC_EXPOSABLE_COLUMNS, CONTENT_PUBLICATION_FIELDS, CONTENT_PUBLICATION_STATUSES, + CONTENT_SEARCH_DESCRIPTION_KINDS, + CONTENT_SEARCH_TEXT_KINDS, + CONTENT_SEARCH_TITLE_KINDS, CONTENT_SYSTEM_FIELDS, } from "./const"; import type { ContentSchemas } from "./schemas"; @@ -484,10 +487,146 @@ export interface ResolvedContentPublicApiConfig< slugField: string; } +// --------------------------------------------------------------------------- +// Search +// --------------------------------------------------------------------------- + +/** Field names of one or more kinds, as a union. */ +type ContentFieldNamesOfKind = string & + { + [K in keyof TFields]: TFields[K] extends { kind: TKind } ? K : never; + }[keyof TFields]; + +/** Field names of one or more kinds that also accept no `null`. */ +type ContentNonNullableFieldNamesOfKind< + TFields, + TKind extends string, +> = string & + { + [K in keyof TFields]: TFields[K] extends { kind: TKind; nullable: false } + ? K + : never; + }[keyof TFields]; + +/** + * Field names `search.titleField` accepts. + * + * Three rules, one `Extract`: `TPublicField` is the public allowlist, so a field + * that is not published cannot be indexed; the kind union keeps prose out of the + * title slot; and the field cannot be nullable, because a `null` heading is not a + * search result. That is why a private field is a compile error and not a lint. + */ +export type ContentSearchTitleField< + TFields, + TPublicField extends string, +> = Extract< + TPublicField, + ContentNonNullableFieldNamesOfKind< + TFields, + (typeof CONTENT_SEARCH_TITLE_KINDS)[number] + > +>; + +export type ContentSearchDescriptionField< + TFields, + TPublicField extends string, +> = Extract< + TPublicField, + ContentFieldNamesOfKind< + TFields, + (typeof CONTENT_SEARCH_DESCRIPTION_KINDS)[number] + > +>; + +export type ContentSearchTextField< + TFields, + TPublicField extends string, +> = Extract< + TPublicField, + ContentFieldNamesOfKind +>; + +/** + * Opts a content type into automatic search synchronization. + * + * Requires `publication` *and* `publicApi`: only published rows are ever + * indexed, and every indexed field has to be publicly readable already - a + * private value would otherwise leak through a result snippet, a highlighted + * match, ranking, or the mere fact that a record matched an exact-match probe. + * + * `enabled` is literal `true` for the same reason publication's and publicApi's + * are: a widened `boolean` would silently resolve to "no search". + * + * Generic over the three field-name *unions* rather than over the field map, so + * `defineContentType` can infer each one from the literal it was given and then + * check it against `ContentSearchTitleField` and friends. Spelling those out + * inside the property types instead looks equivalent and is not: `TPublicField` + * falls back to its constraint while the argument that infers it is still being + * checked, and a private field name would slip through. + */ +export interface ContentSearchConfig< + TTitle extends string = string, + TDescription extends string = string, + TText extends string = string, +> { + /** Concatenated into the indexed body, in order. At least one. */ + contentFields: readonly [TText, ...TText[]]; + /** Prepended to the indexed body so it shows up in result excerpts. */ + descriptionField?: TDescription; + enabled: true; + /** + * The public URL of one record, e.g. `/articles/{slug}`. Relative, and + * `{slug}` - the exposed slug field - is the only placeholder. + */ + pathTemplate: string; + /** The result heading. Weighted above the body by the index. */ + titleField: TTitle; +} + +/** + * Whether a `search` argument opted in. + * + * `defineContentType` infers the whole `search` object as one type parameter - + * an intersection member like `{ enabled: TEnabled }` is not an inference site, + * so reading the literal back off the argument is the only way to keep it. + */ +export type ContentSearchEnabled = TSearch extends { enabled: true } + ? true + : false; + +/** + * `search` after `defineContentType` has filled in every default. + * + * Generic over `enabled` for the same reason `publication` and `publicApi` are: + * a widened `boolean` would make every definition equally (un)searchable, so + * `SearchableContentTypeDefinition` would only ever match after a cast. + */ +export interface ResolvedContentSearchConfig< + TEnabled extends boolean = boolean, +> { + contentFields: string[]; + descriptionField: null | string; + enabled: TEnabled; + pathTemplate: string; + titleField: string; +} + // --------------------------------------------------------------------------- // Definition // --------------------------------------------------------------------------- +/** + * A content type whose records are synchronized with the search index. + * + * An intersection rather than a sixth type argument, for the same reason + * {@link PublicContentTypeDefinition} is one: `enabled` is the only thing a + * caller of the search layer needs pinned, and narrowing just that keeps every + * concrete definition assignable. + */ +export type SearchableContentTypeDefinition = AnyContentTypeDefinition & { + search: { enabled: true }; +}; + /** * A content type that actually has a generated public API. * @@ -511,6 +650,7 @@ export interface ContentTypeDefinition< TPublication extends boolean = boolean, TPublicField extends string = string, TPublicEnabled extends boolean = boolean, + TSearchEnabled extends boolean = boolean, > { admin: ResolvedContentAdminConfig; fields: TFields; @@ -528,9 +668,12 @@ export interface ContentTypeDefinition< TFields, TPublication, TPublicField, - TPublicEnabled + TPublicEnabled, + TSearchEnabled > >; + /** Search synchronization, or the disabled default when `search` is omitted. */ + search: ResolvedContentSearchConfig; tableName: string; } diff --git a/packages/vitnode/src/locales/en.json b/packages/vitnode/src/locales/en.json index b6c3bf39b..c0c9c3bed 100644 --- a/packages/vitnode/src/locales/en.json +++ b/packages/vitnode/src/locales/en.json @@ -83,6 +83,10 @@ "rebuildError": "Could not queue the rebuild.", "reindex": "Reindex", "reindexQueued": "Reindex queued for {collection}.", + "syncErrors": { + "title": "Recent sync failures", + "desc": "These records were saved, but their search documents weren't updated. Rebuild the index to repair them." + }, "cron": { "title": "Background jobs require a configured cron adapter", "desc": "A rebuild won't run until cron is active. Configure an adapter in Integrations to enable scheduled reindexing.", @@ -113,8 +117,17 @@ "status": { "indexed": "Indexed", "stale": "Stale", - "empty": "Not indexed" + "empty": "Not indexed", + "unmanaged": "Unmanaged" }, + "noIndexer": "No rebuild indexer", + "noIndexerDesc": "No rebuild indexer is registered. Documents may still be maintained by live plugin writes.", + "removeDocuments": "Remove documents", + "removeConfirmTitle": "Remove indexed documents for \u201c{collection}\u201d?", + "removeConfirmDescription": "No rebuild indexer is registered for this collection. Its documents may still be maintained by live plugin writes. This removes their current indexed state, and future writes may add them again.", + "removeSuccess": "Removed the indexed documents for {collection}.", + "removeSuccessDesc": "Live plugin writes may recreate them.", + "removeError": "Could not remove the indexed documents.", "empty": "No collections found", "emptyDesc": "No collection matches your search. Try a different term." } diff --git a/packages/vitnode/src/tests/content-fixtures.ts b/packages/vitnode/src/tests/content-fixtures.ts index 162879350..7ee988755 100644 --- a/packages/vitnode/src/tests/content-fixtures.ts +++ b/packages/vitnode/src/tests/content-fixtures.ts @@ -97,3 +97,44 @@ export const testPostContentType = defineContentType({ }, }, }); + +/** + * The Stage 3 shape: `testPostContentType` plus `search`. + * + * A separate fixture rather than a flag on the post: keeping the post exactly as + * it was is what proves a Stage 2 content type is untouched by search existing. + * `code` and `author` stay out of `publicApi.fields` so "an indexed field is a + * public field" has something to be wrong about. + */ +export const testSearchablePostContentType = defineContentType({ + id: "test.searchable", + tableName: "test_searchable_posts", + fields: { + title: field.text({ required: true, minLength: 3, maxLength: 200 }), + slug: field.slug({ source: "title" }), + excerpt: field.textarea({ maxLength: 500, nullable: true }), + body: field.textarea({ nullable: true }), + code: field.text({ required: true, maxLength: 100 }), + views: field.number({ integer: true, min: 0, defaultValue: 0 }), + author: field.user(), + }, + publication: { enabled: true }, + publicApi: { + enabled: true, + path: "searchable", + fields: ["title", "slug", "excerpt", "body", "publishedAt"], + defaultOrderBy: "publishedAt", + }, + search: { + enabled: true, + titleField: "title", + descriptionField: "excerpt", + contentFields: ["excerpt", "body"], + pathTemplate: "/searchable/{slug}", + }, + admin: { + label: { plural: "Test Searchables", singular: "Test Searchable" }, + titleField: "title", + list: { defaultOrderBy: "publishedAt" }, + }, +}); diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/collection-label.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-label.ts new file mode 100644 index 000000000..69dadb6da --- /dev/null +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/collection-label.ts @@ -0,0 +1,18 @@ +import { getFrontendContentTypes } from "@/content/admin/config"; + +/** + * Human labels for the collections a Content Engine content type contributes. + * + * A generated indexer uses the content type id as its item type + * (`example.article`), and the renderer registry in `views/search/registry` is a + * hardcoded core map - so without this every content collection would read + * "Content". Resolved from the frontend registry, which is why this is called + * from the server component and not from the shared table. + */ +export const getContentCollectionLabels = (): Map => + new Map( + getFrontendContentTypes().map(({ definition }) => [ + definition.id, + definition.admin.label.plural, + ]), + ); 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 4c29342c0..1c7bd2bf0 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 @@ -2,39 +2,118 @@ import { describe, expect, it } from "vitest"; import { getCollectionCoverage, + getCollectionCoverageBar, getCollectionStatus, } from "./collection-status"; +/** A registered collection: an indexer exists, so `total` is a real count. */ +const registered = (indexed: number, total: number) => ({ + hasIndexer: true, + indexed, + total, +}); + +/** + * A collection with no rebuild indexer, and so no source count. It may still be + * kept current by live `search.index()` writes - that is why the status only + * claims it is outside the rebuild system. + */ +const unmanaged = (indexed: number) => ({ + hasIndexer: false, + indexed, + total: null, +}); + describe("getCollectionStatus", () => { it("reports empty when nothing is indexed", () => { - expect(getCollectionStatus({ indexed: 0, total: 12 })).toBe("empty"); - expect(getCollectionStatus({ indexed: 0, total: 0 })).toBe("empty"); + expect(getCollectionStatus(registered(0, 0))).toBe("empty"); + expect(getCollectionStatus(registered(0, 10))).toBe("empty"); }); it("reports stale when fewer items are indexed than the source has", () => { - expect(getCollectionStatus({ indexed: 6, total: 8 })).toBe("stale"); + expect(getCollectionStatus(registered(5, 10))).toBe("stale"); + }); + + it("reports indexed only when the counts match exactly", () => { + expect(getCollectionStatus(registered(10, 10))).toBe("indexed"); }); - it("reports indexed when coverage is complete", () => { - expect(getCollectionStatus({ indexed: 2, total: 2 })).toBe("indexed"); + it("reports stale when more items are indexed than the source has", () => { + // Documents surviving for records that no longer qualify. Calling this + // healthy is how a stale index stays invisible. + expect(getCollectionStatus(registered(11, 10))).toBe("stale"); + expect(getCollectionStatus(registered(10, 0))).toBe("stale"); }); - it("never reports stale when the indexed count exceeds the source count", () => { - expect(getCollectionStatus({ indexed: 5, total: 3 })).toBe("indexed"); + it("never calls an over-indexed collection healthy", () => { + for (const indexed of [1, 2, 11, 100]) { + expect(getCollectionStatus(registered(indexed, 0))).not.toBe("indexed"); + } + expect(getCollectionStatus(registered(11, 10))).not.toBe("indexed"); + }); + + describe("collections with no rebuild indexer", () => { + it("reports unmanaged when documents have no indexer", () => { + expect(getCollectionStatus(unmanaged(11))).toBe("unmanaged"); + }); + + it("decides on indexer availability before comparing counts", () => { + // The trap this exists for: the old `total = indexed` fallback made + // `indexed === total` true, so an unrebuildable collection read as healthy. + expect( + getCollectionStatus({ hasIndexer: false, indexed: 11, total: 11 }), + ).toBe("unmanaged"); + expect( + getCollectionStatus({ hasIndexer: false, indexed: 1, total: 1 }), + ).toBe("unmanaged"); + }); + + it("reports empty rather than unmanaged when there is nothing indexed", () => { + // Nothing indexed, so nothing to say about how it would be rebuilt. + expect(getCollectionStatus(unmanaged(0))).toBe("empty"); + }); }); }); describe("getCollectionCoverage", () => { it("returns a whole-percent ratio of indexed to total", () => { - expect(getCollectionCoverage({ indexed: 6, total: 8 })).toBe(75); - expect(getCollectionCoverage({ indexed: 2, total: 2 })).toBe(100); + expect(getCollectionCoverage(registered(5, 10))).toBe(50); + expect(getCollectionCoverage(registered(10, 10))).toBe(100); }); it("returns 0 when there is nothing to cover", () => { - expect(getCollectionCoverage({ indexed: 0, total: 0 })).toBe(0); + expect(getCollectionCoverage(registered(0, 0))).toBe(0); + expect(getCollectionCoverage(registered(0, 10))).toBe(0); }); it("rounds to the nearest percent", () => { - expect(getCollectionCoverage({ indexed: 1, total: 3 })).toBe(33); + expect(getCollectionCoverage(registered(1, 3))).toBe(33); + }); + + it("reports past 100 rather than hiding an over-indexed collection", () => { + expect(getCollectionCoverage(registered(11, 10))).toBe(110); + expect(getCollectionCoverage(registered(10, 0))).toBe(100); + }); + + it("returns null when there is no source count", () => { + // Not 100: there is nothing to be complete against. + expect(getCollectionCoverage(unmanaged(11))).toBeNull(); + expect(getCollectionCoverage(unmanaged(0))).toBeNull(); + }); +}); + +describe("getCollectionCoverageBar", () => { + it("clamps the drawn width to the track", () => { + expect(getCollectionCoverageBar(registered(11, 10))).toBe(100); + expect(getCollectionCoverageBar(registered(200, 10))).toBe(100); + }); + + it("matches the measured coverage below the cap", () => { + expect(getCollectionCoverageBar(registered(5, 10))).toBe(50); + expect(getCollectionCoverageBar(registered(0, 10))).toBe(0); + }); + + it("draws nothing without a source count", () => { + expect(getCollectionCoverageBar(unmanaged(11))).toBeNull(); }); }); 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 eb9a75628..875d985e8 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,27 +1,67 @@ export interface SearchCollection { + /** Whether a rebuild indexer is registered for this item type right now. */ + hasIndexer: boolean; indexed: number; itemType: string; lastIndexedAt: Date | null | string; pluginId: string; - total: number; + /** Source items the indexer reports; `null` when there is no indexer. */ + total: null | number; } -export type CollectionStatus = "empty" | "indexed" | "stale"; +export type CollectionStatus = "empty" | "indexed" | "stale" | "unmanaged"; -// Nothing indexed yet, partially indexed (fewer items than the source has), or -// fully covered - the three states the coverage report distinguishes. +/** + * Nothing indexed, exactly covered, out of step, or outside the rebuild system. + * + * "Out of step" is any mismatch, in **either** direction. Fewer documents than + * source records means something was missed; more means documents survive for + * records that no longer qualify - and calling that one healthy is how a stale + * index stays invisible. + * + * "Unmanaged" comes first and does not look at the counts at all: without an + * indexer there is no source to compare against, so `indexed` matching `total` + * would only mean the fallback matched itself. It says nothing about the plugin - + * registering an indexer is optional, and a plugin that only ever calls + * `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): CollectionStatus => { +}: Pick< + SearchCollection, + "hasIndexer" | "indexed" | "total" +>): CollectionStatus => { + if (!hasIndexer && indexed > 0) return "unmanaged"; if (indexed === 0) return "empty"; - if (indexed < total) return "stale"; + if (indexed === total) return "indexed"; - return "indexed"; + return "stale"; }; +/** + * Indexed items as a percentage of source items, or `null` when there is no + * source count to divide by. + * + * 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): number => - total > 0 ? Math.round((indexed / total) * 100) : 0; +}: Pick): null | number => { + if (total === null) return null; + if (total > 0) return Math.round((indexed / total) * 100); + + return indexed > 0 ? 100 : 0; +}; + +export const getCollectionCoverageBar = ( + collection: Pick, +): null | number => { + const coverage = getCollectionCoverage(collection); + + return coverage === null ? null : Math.min(coverage, 100); +}; 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 d55bc2ce0..1842aa4ea 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 @@ -19,9 +19,11 @@ import type { CollectionStatus, SearchCollection } from "./collection-status"; import { getCollectionCoverage, + getCollectionCoverageBar, getCollectionStatus, } from "./collection-status"; import { ReindexCollectionAction } from "./reindex-action"; +import { RemoveCollectionDocumentsAction } from "./remove-documents-action"; interface CollectionRow extends SearchCollection { id: number; @@ -47,13 +49,21 @@ const statusStyles: Record< dot: "bg-muted-foreground/50", text: "text-muted-foreground", }, + unmanaged: { + bar: "bg-destructive", + dot: "bg-destructive", + text: "text-destructive", + }, }; export const CollectionsTable = async ({ collections, + labels, search, }: { collections: SearchCollection[]; + /** Content type id -> label, for collections the renderer registry has no entry for. */ + labels?: Map; search?: string; }) => { const t = await getTranslations("core.search"); @@ -62,7 +72,9 @@ export const CollectionsTable = async ({ .map((collection, index) => ({ ...collection, id: index, - label: t(getSearchTypeRenderer(collection.itemType).labelKey), + label: + labels?.get(collection.itemType) ?? + t(getSearchTypeRenderer(collection.itemType).labelKey), })) .sort((a, b) => b.indexed - a.indexed || a.label.localeCompare(b.label)); @@ -98,7 +110,19 @@ export const CollectionsTable = async ({ > {t(`admin.collections.status.${status}`)} + + · + {row.pluginId} + + {status === "unmanaged" && ( + // Says only what is known. A plugin may index live and never + // register a rebuild indexer, so "unmanaged" is about the + // rebuild system - not about the plugin being gone. +

+ {t("admin.collections.noIndexerDesc")} +

+ )} ); @@ -110,7 +134,10 @@ export const CollectionsTable = async ({ cell: ({ row }) => ( {row.indexed} - / {row.total} + {/* 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. */} + / {row.total ?? "—"} ), }, @@ -120,17 +147,28 @@ export const CollectionsTable = async ({ className: "w-52", cell: ({ row }) => { const coverage = getCollectionCoverage(row); + const bar = getCollectionCoverageBar(row); const styles = statusStyles[getCollectionStatus(row)]; + if (coverage === null || bar === null) { + return ( + + {t("admin.collections.noIndexer")} + + ); + } + return (
- + {coverage}%
@@ -154,9 +192,17 @@ export const CollectionsTable = async ({ id: "actions", header: "", align: "right", - cell: ({ row }) => ( - - ), + cell: ({ row }) => + row.hasIndexer ? ( + + ) : ( + // Rebuilding this would clear it and refill nothing, so the only offer + // is the honest one: remove what is currently indexed. + + ), }, ]; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/mutation-api.server.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/mutation-api.server.ts index 91add929b..471d5aaf6 100644 --- a/packages/vitnode/src/views/admin/views/core/advanced/search/mutation-api.server.ts +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/mutation-api.server.ts @@ -20,3 +20,26 @@ export const rebuildSearchIndexMutation = async (itemType?: string) => { return { data: await res.json() }; }; + +/** + * Removes the documents of a collection with no rebuild indexer. Destructive: + * nothing *rebuilds* them afterwards, though the owning plugin may write them + * again live. The API refuses it for any collection that has an indexer. + */ +export const clearSearchCollectionMutation = async (itemType: string) => { + const res = await fetcher(debugAdminModule, { + prefixPath: "/admin", + path: "/search/clear", + method: "post", + module: "debug", + args: { + body: { itemType }, + }, + }); + + if (!res.ok) { + return { error: await res.text() }; + } + + return { data: await res.json() }; +}; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/remove-documents-action.tsx b/packages/vitnode/src/views/admin/views/core/advanced/search/remove-documents-action.tsx new file mode 100644 index 000000000..50296f3c0 --- /dev/null +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/remove-documents-action.tsx @@ -0,0 +1,60 @@ +"use client"; + +import { Trash2Icon } from "lucide-react"; +import { useTranslations } from "next-intl"; +import { toast } from "sonner"; + +import { ConfirmActionAlertDialog } from "@/components/confirm-action/confirm-action-alert-dialog"; +import { Button } from "@/components/ui/button"; +import { useRouter } from "@/lib/navigation"; + +import { clearSearchCollectionMutation } from "./mutation-api.server"; + +/** + * The only way to clear a collection with no rebuild indexer, and deliberately + * not called "reindex": nothing *rebuilds* these documents afterwards. + * + * They may still come back. Registering an indexer is optional, so the owning + * plugin may be writing them live through `search.index()` - which is why the + * confirmation says the current indexed state is what goes, and not that the + * documents are gone for good. + */ +export const RemoveCollectionDocumentsAction = ({ + itemType, + label, +}: { + itemType: string; + label: string; +}) => { + const t = useTranslations("core.search.admin.collections"); + const router = useRouter(); + + return ( + { + const result = await clearSearchCollectionMutation(itemType); + + if (result.error) { + toast.error(t("removeError"), { description: label }); + + return; + } + + toast.success(t("removeSuccess", { collection: label }), { + description: t("removeSuccessDesc"), + }); + onClose(); + router.refresh(); + }} + submitVariant="destructive" + textSubmit={t("removeDocuments")} + title={t("removeConfirmTitle", { collection: label })} + > + + + ); +}; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/search-view.tsx b/packages/vitnode/src/views/admin/views/core/advanced/search/search-view.tsx index 58fc12f0b..c27e4c2d4 100644 --- a/packages/vitnode/src/views/admin/views/core/advanced/search/search-view.tsx +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/search-view.tsx @@ -15,8 +15,10 @@ import { Skeleton } from "@/components/ui/skeleton"; import { fetcher } from "@/lib/fetcher"; import { cn } from "@/lib/utils"; +import { getContentCollectionLabels } from "./collection-label"; import { CollectionsTable } from "./collections-table"; import { CronWarning } from "./cron-warning"; +import { SyncErrorsCard } from "./sync-errors-card"; const getStatus = async () => { const res = await fetcher(debugAdminModule, { @@ -78,6 +80,11 @@ export const SearchAdminView = async ({ searchParams, ]); + // Content Engine collections use the content type id as their item type, which + // the renderer registry has no entry for - resolve their labels here, where the + // frontend content type registry is readable. + const labels = getContentCollectionLabels(); + return (
{!data.hasCronAdapter && } @@ -138,7 +145,13 @@ export const SearchAdminView = async ({ />
- + + +
); }; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors-card.tsx b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors-card.tsx new file mode 100644 index 000000000..b6e92e76c --- /dev/null +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors-card.tsx @@ -0,0 +1,86 @@ +import { TriangleAlertIcon } from "lucide-react"; +import { getTranslations } from "next-intl/server"; + +import { DateFormat } from "@/components/date-format"; +import { + Card, + CardContent, + CardDescription, + CardHeader, + CardTitle, +} from "@/components/ui/card"; + +import type { SearchSyncError } from "./sync-errors"; + +import { parseSearchSyncError } from "./sync-errors"; + +/** + * The last few times a content mutation succeeded but its search document did + * not follow. + * + * Content Engine synchronization is best effort by design: a temporary search + * engine outage must not turn a committed write into a failed one. This panel is + * how that tradeoff stays visible, and a rebuild is how it gets repaired. + */ +export const SyncErrorsCard = async ({ + errors, + labels, +}: { + errors: SearchSyncError[]; + labels?: Map; +}) => { + if (errors.length === 0) return null; + + const t = await getTranslations("core.search.admin.syncErrors"); + + return ( + + + + + {t("title")} + + {t("desc")} + + +
    + {errors.map(error => { + const parsed = parseSearchSyncError(error.content); + const collection = parsed.contentTypeId + ? (labels?.get(parsed.contentTypeId) ?? parsed.contentTypeId) + : error.pluginId; + + return ( +
  • +
    + + {collection} + {parsed.operation ? ( + + {" "} + · {parsed.operation} + {parsed.documentId ? ` · ${parsed.documentId}` : ""} + + ) : null} + + + + +
    +

    + {parsed.message ?? error.content} +

    +
    • + ); + })} +
+ + + ); +}; diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.test.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.test.ts new file mode 100644 index 000000000..9cb763a4f --- /dev/null +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; + +import { parseSearchSyncError } from "./sync-errors"; + +describe("parseSearchSyncError", () => { + it("reads the structured payload out of a log line", () => { + expect( + parseSearchSyncError( + '[content-search] {"action":"upsert","contentTypeId":"example.article","documentId":"example.article:7","error":"engine unavailable","itemId":7,"operation":"publish"}', + ), + ).toEqual({ + contentTypeId: "example.article", + documentId: "example.article:7", + message: "engine unavailable", + operation: "publish", + }); + }); + + it("falls back to nulls for a line with no payload", () => { + expect( + parseSearchSyncError("[content-search] something went wrong"), + ).toEqual({ + contentTypeId: null, + documentId: null, + message: null, + operation: null, + }); + }); + + it("falls back to nulls for malformed JSON", () => { + expect( + parseSearchSyncError("[content-search] {not json").message, + ).toBeNull(); + }); + + it("ignores non-string values", () => { + expect( + parseSearchSyncError('[content-search] {"contentTypeId":7,"error":""}'), + ).toEqual({ + contentTypeId: null, + documentId: null, + message: null, + operation: null, + }); + }); +}); diff --git a/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.ts b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.ts new file mode 100644 index 000000000..0aff39c6b --- /dev/null +++ b/packages/vitnode/src/views/admin/views/core/advanced/search/sync-errors.ts @@ -0,0 +1,46 @@ +export interface SearchSyncError { + content: string; + createdAt: Date | string; + id: number; + pluginId: string; +} + +export interface ParsedSearchSyncError { + contentTypeId: null | string; + documentId: null | string; + message: null | string; + operation: null | string; +} + +const asString = (value: unknown): null | string => + typeof value === "string" && value !== "" ? value : null; + +export const parseSearchSyncError = ( + content: string, +): ParsedSearchSyncError => { + const empty: ParsedSearchSyncError = { + contentTypeId: null, + documentId: null, + message: null, + operation: null, + }; + + const start = content.indexOf("{"); + if (start === -1) return empty; + + try { + const parsed: unknown = JSON.parse(content.slice(start)); + if (typeof parsed !== "object" || parsed === null) return empty; + + const values = parsed as Record; + + return { + contentTypeId: asString(values.contentTypeId), + documentId: asString(values.documentId), + message: asString(values.error), + operation: asString(values.operation), + }; + } catch { + return empty; + } +}; diff --git a/plugins/blog/src/api/lib/search.ts b/plugins/blog/src/api/lib/search.ts index e36ba9702..bf70575e4 100644 --- a/plugins/blog/src/api/lib/search.ts +++ b/plugins/blog/src/api/lib/search.ts @@ -121,7 +121,7 @@ export const blogPostSearchIndexer: SearchIndexer = { .offset(offset); if (rows.length === 0) { - return []; + return { documents: [], itemsRead: 0 }; } const languageCodes = getEnabledLanguageCodes(c); @@ -133,13 +133,18 @@ export const blogPostSearchIndexer: SearchIndexer = { ), ]); - return rows.flatMap(post => - buildDocumentsForPost( - post, - languageCodes, - translations.get(post.id), - defaultLanguageCode, + // One post emits one document per enabled language, so the document count is + // never the source count - `itemsRead` is what the rebuild pages by. + return { + documents: rows.flatMap(post => + buildDocumentsForPost( + post, + languageCodes, + translations.get(post.id), + defaultLanguageCode, + ), ), - ); + itemsRead: rows.length, + }; }, }; diff --git a/plugins/example/src/content/article.ts b/plugins/example/src/content/article.ts index e6eaaec3c..69fab5094 100644 --- a/plugins/example/src/content/article.ts +++ b/plugins/example/src/content/article.ts @@ -54,6 +54,21 @@ export const articleContentType = defineContentType({ defaultOrder: "desc", }, + // Published articles are kept in the site-wide search index automatically: + // publishing adds the document, editing an indexed field or the slug rewrites + // it, unpublishing and deleting remove it. Drafts are never indexed. + // + // Every field named here is also in `publicApi.fields` - that is enforced by + // the types, not just by review. Naming `code` or `author` would not compile, + // which is what stops a private value surfacing in a result snippet. + search: { + enabled: true, + titleField: "title", + descriptionField: "excerpt", + contentFields: ["title", "excerpt"], + pathTemplate: "/articles/{slug}", + }, + // The generated columns are addressable here too. `(status, publishedAt)` is // generated automatically; this one backs "newest drafts first". indexes: [{ on: ["status", "createdAt"] }], diff --git a/plugins/example/src/database/postgres.test.ts b/plugins/example/src/database/postgres.test.ts index b81e566bb..d2ae33ea9 100644 --- a/plugins/example/src/database/postgres.test.ts +++ b/plugins/example/src/database/postgres.test.ts @@ -1,5 +1,10 @@ +import type { ContentSearchOperation } from "@vitnode/core/content/server"; import type { Context } from "hono"; +import { + createContentSearchIndexer, + syncContentSearch, +} from "@vitnode/core/content/server"; import { drizzle } from "drizzle-orm/postgres-js"; import { readFileSync } from "node:fs"; import { dirname, resolve } from "node:path"; @@ -7,7 +12,8 @@ import { fileURLToPath } from "node:url"; import postgres from "postgres"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { EXAMPLE_MIGRATIONS } from "@/const"; +import { CONFIG_PLUGIN, EXAMPLE_MIGRATIONS } from "@/const"; +import { articleContentType } from "@/content/article"; import { articleContent } from "./articles"; import { categoryContent } from "./categories"; @@ -123,6 +129,7 @@ const CORE_USERS_STUB = ` let sql: ReturnType; let context: Context; +let db: ReturnType; let serverMajor = 0; const pgErrorCode = async (run: () => Promise) => { @@ -202,9 +209,9 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { ) `; + db = drizzle(sql, { casing: "camelCase" }); context = { - get: (key: string) => - key === "db" ? drizzle(sql, { casing: "camelCase" }) : undefined, + get: (key: string) => (key === "db" ? db : undefined), } as unknown as Context; }, 60_000); @@ -739,6 +746,288 @@ describe.skipIf(!url)("Content Engine against Postgres", () => { await categories.delete(category.id); }, 60_000); + it("runs the whole search lifecycle", async () => { + const categories = categoryContent.service(context); + const articles = articleContent.service(context); + const indexer = createContentSearchIndexer(articleContent, { + pluginId: CONFIG_PLUGIN.pluginId, + }); + + // The search engine is the one thing stubbed here: `core_search_index` is a + // core table whose generated column needs text-search configurations a stock + // Postgres image does not ship (the same reason `core_users` is a stub). The + // publication state every decision below turns on is real. + const calls: { args: unknown[]; op: "delete" | "index" }[] = []; + const searchContext = { + get: (key: string) => { + if (key === "search") { + return { + delete: async (...args: unknown[]) => { + calls.push({ args, op: "delete" }); + await Promise.resolve(); + }, + index: async (...args: unknown[]) => { + calls.push({ args, op: "index" }); + await Promise.resolve(); + }, + }; + } + if (key === "log") { + return { + debug: async () => await Promise.resolve(), + error: async () => await Promise.resolve(), + warn: async () => await Promise.resolve(), + }; + } + + return db; + }, + } as unknown as Context; + + const sync = async ( + operation: ContentSearchOperation, + input: { changed?: boolean; changedFields?: string[]; row: object }, + ) => + await syncContentSearch(searchContext, articleContentType, { + operation, + // A direct caller passes its own plugin id, exactly as the generated + // routes do - otherwise the document would be owned by whichever plugin + // the request belongs to. + pluginId: CONFIG_PLUGIN.pluginId, + ...input, + }); + + const category = await categories.create({ name: "Searchable" }); + const draft = await articles.create({ + category: category.id, + code: "search-001", + excerpt: "A findable summary", + title: "Findable article", + }); + + // A draft is never indexed, and the decision is made from the real row. + await expect(sync("create", { row: draft })).resolves.toMatchObject({ + action: "skip", + }); + await expect(indexer.count?.(context)).resolves.toBe(0); + await expect(indexer.load(context, 0, 10)).resolves.toEqual({ + documents: [], + itemsRead: 0, + }); + + const published = await articles.publish(draft.id); + if (!published) throw new Error("Expected a publish result."); + + const upsert = await sync("publish", { + changed: published.changed, + row: published.row, + }); + expect(upsert.action).toBe("upsert"); + expect(upsert.documentId).toBe(`example.article:${draft.id}`); + expect(calls.at(-1)?.op).toBe("index"); + expect(calls.at(-1)?.args[0]).toMatchObject({ + // `title` is one of the example's `contentFields`, and `excerpt` is both + // the description and the second one - so it appears once, not twice. + content: "Findable article\n\nA findable summary", + isPublic: true, + itemId: draft.id, + itemType: "example.article", + // The owning plugin, not core - the same value a rebuild stamps. + pluginId: CONFIG_PLUGIN.pluginId, + title: "Findable article", + url: "/articles/findable-article", + }); + + // Only published rows reach the rebuild, and only the projected columns. + await expect(indexer.count?.(context)).resolves.toBe(1); + const page = await indexer.load(context, 0, 10); + expect(page.itemsRead).toBe(1); + const [document] = page.documents; + expect(document).toMatchObject({ + itemId: draft.id, + itemType: "example.article", + // A rebuild reproduces the ownership the live write stored. + pluginId: CONFIG_PLUGIN.pluginId, + url: "/articles/findable-article", + }); + // The private columns are not in the document, and the author is not in it + // either - a user field is never public, so it is never indexed. + expect(JSON.stringify(document)).not.toContain("search-001"); + expect(document).not.toHaveProperty("authorId"); + + // Offsets page over the published rows deterministically, and the cursor + // advances by source rows read. + await expect(indexer.load(context, 1, 10)).resolves.toEqual({ + documents: [], + itemsRead: 0, + }); + + // An idempotent publish writes nothing. + const noop = await articles.publish(draft.id); + expect(noop?.changed).toBe(false); + const before = calls.length; + await expect( + sync("publish", { changed: noop?.changed, row: noop?.row ?? {} }), + ).resolves.toMatchObject({ action: "skip" }); + expect(calls).toHaveLength(before); + + // An update that moves no indexed field writes nothing. + const bumped = await articles.update(draft.id, { views: 5 }); + await expect( + sync("update", { + changedFields: bumped?.changedFields, + row: bumped?.row ?? {}, + }), + ).resolves.toMatchObject({ action: "skip" }); + + // A slug change rewrites the url in place - there is no stale document, + // because the document is keyed by item type and id. + const renamed = await articles.update(draft.id, { slug: "moved-article" }); + await expect( + sync("update", { + changedFields: renamed?.changedFields, + row: renamed?.row ?? {}, + }), + ).resolves.toMatchObject({ action: "upsert" }); + expect(calls.at(-1)?.args[0]).toMatchObject({ + url: "/articles/moved-article", + }); + await expect(indexer.load(context, 0, 10)).resolves.toMatchObject({ + documents: [{ url: "/articles/moved-article" }], + itemsRead: 1, + }); + + // Unpublishing removes the document and takes the row out of the rebuild. + const unpublished = await articles.unpublish(draft.id); + await expect( + sync("unpublish", { + changed: unpublished?.changed, + row: unpublished?.row ?? {}, + }), + ).resolves.toMatchObject({ action: "delete" }); + expect(calls.at(-1)).toMatchObject({ + args: ["example.article", draft.id], + op: "delete", + }); + await expect(indexer.count?.(context)).resolves.toBe(0); + await expect(indexer.load(context, 0, 10)).resolves.toEqual({ + documents: [], + itemsRead: 0, + }); + + // A future publication date is not public, so it is not indexed either - + // the same `publishedAt <= now()` rule the public read layer uses. + await articles.publish(draft.id); + await sql` + UPDATE "example_articles" + SET "publishedAt" = now() + interval '1 day' + WHERE "id" = ${draft.id} + `; + await expect(indexer.load(context, 0, 10)).resolves.toEqual({ + documents: [], + itemsRead: 0, + }); + const scheduled = await articles.findById(draft.id); + await expect( + sync("update", { changedFields: ["title"], row: scheduled ?? {} }), + ).resolves.toMatchObject({ action: "skip" }); + + await sql`UPDATE "example_articles" SET "publishedAt" = now() WHERE "id" = ${draft.id}`; + + // Deleting a published record removes its document; `publishedAt` survives + // an unpublish, so a record that was ever published is cleaned up too. + const deleted = await articles.delete(draft.id); + await expect(sync("delete", { row: deleted ?? {} })).resolves.toMatchObject( + { action: "delete" }, + ); + + const neverPublished = await articles.create({ + category: category.id, + code: "search-002", + title: "Never published", + }); + await expect( + sync("delete", { row: neverPublished }), + ).resolves.toMatchObject({ action: "skip" }); + + await articles.delete(neverPublished.id); + await categories.delete(category.id); + }, 60_000); + + it("keeps paging past published rows it cannot project", async () => { + const categories = categoryContent.service(context); + const articles = articleContent.service(context); + const indexer = createContentSearchIndexer(articleContent, { + pluginId: CONFIG_PLUGIN.pluginId, + }); + + const category = await categories.create({ name: "Paging" }); + const created: { id: number }[] = []; + for (const index of [1, 2, 3]) { + const article = await articles.create({ + category: category.id, + code: `paging-00${index}`, + title: `Paging article ${index}`, + }); + await articles.publish(article.id); + created.push(article); + } + + // Only the database can produce this: `title` is required and non-nullable, + // so the engine never writes a blank one. It is still what a rebuild has to + // survive - the row is published, and the mapper refuses it. + await sql` + UPDATE "example_articles" SET "title" = ' ' WHERE "id" = ${created[0].id} + `; + + // Page one reads a row and projects nothing. `itemsRead` is what says the + // source is not finished, which is the whole reason it is reported. + const first = await indexer.load(context, 0, 1); + expect(first.itemsRead).toBe(1); + expect(first.documents).toEqual([]); + + const second = await indexer.load(context, first.itemsRead, 1); + expect(second.itemsRead).toBe(1); + expect(second.documents.map(document => document.itemId)).toEqual([ + created[1].id, + ]); + + // The source count still counts the malformed row, so the collection reads + // as under-indexed in the AdminCP rather than silently complete. + await expect(indexer.count?.(context)).resolves.toBe(3); + const all = await indexer.load(context, 0, 50); + expect(all.itemsRead).toBe(3); + expect(all.documents).toHaveLength(2); + + for (const article of created) await articles.delete(article.id); + await categories.delete(category.id); + }, 60_000); + + it("adds no columns or indexes for search", async () => { + // Search is a projection of columns that already exist. If it ever needed + // one of its own, every content type opting in would need a migration. + const columns = await sql<{ column_name: string }[]>` + SELECT column_name FROM information_schema.columns + WHERE table_name = 'example_articles' + `; + + expect(columns.map(row => row.column_name).sort()).toEqual([ + "author", + "category", + "code", + "createdAt", + "excerpt", + "featured", + "id", + "publishedAt", + "slug", + "status", + "title", + "updatedAt", + "views", + ]); + }); + it("has no public service without a public API", () => { // `example.category` opts into neither publication nor `publicApi`. expect(categoryContent.publicService).toBeUndefined();