github-mirror/oxynote
1
0
Fork 0
mirror of https://github.com/oxynote/oxynote.git synced 2026-10-17 02:49:18 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

add branch support to search

Browse Source
This commit is contained in:
david 2026-09-04 00:18:39 +03:00
parent 4bcd34bf6b
commit 9b0bd2fea4
27 changed files with 979 additions and 288 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
docker/docker-compose.dev.yaml
View File

@ -72,7 +72,7 @@ services:
postgres:
condition: service_healthy
meilisearch:
condition: service_started
condition: service_healthy
volumes:
# optional GitHub App private key: drop oxynote.private-key.pem into
# docker/github/ and set OXYNOTE_CORE_GITHUB_APP_ID to enable the
@ -164,6 +164,12 @@ services:
- MEILI_MASTER_KEY=devmasterkey
- MEILI_ENV=development
- MEILI_LOG_LEVEL=WARN
- MEILI_NO_ANALYTICS=true
healthcheck:
test: ["CMD", "curl", "--fail", "http://localhost:7700/health"]
interval: 5s
timeout: 5s
retries: 20
restart: unless-stopped
logging:
driver: local

1
docker/prod/docker-compose.example.yaml
View File

@ -66,6 +66,7 @@ services:
- MEILI_ENV=production
- MEILI_MASTER_KEY=change-me-meili-key
- MEILI_LOG_LEVEL=WARN
- MEILI_NO_ANALYTICS=true
volumes:
- meilisearch_data:/meili_data
healthcheck:

1
e2e/docker-compose.dev.yaml
View File

@ -150,6 +150,7 @@ services:
- MEILI_MASTER_KEY=devmasterkey
- MEILI_ENV=development
- MEILI_LOG_LEVEL=WARN
- MEILI_NO_ANALYTICS=true
healthcheck:
test: ["CMD", "curl", "--fail", "http://localhost:7700/health"]
interval: 5s

101
e2e/tests/search.test.ts
View File

@ -8,6 +8,15 @@ import {
waitForEditor,
} from "../helpers/editor"
import { t } from "../helpers/i18n"
import { documentId } from "../helpers/api"
import { authorizeMCPClient, connectMCPClient } from "../helpers/mcp"
import {
branchLabel,
branchSwitcher,
makeReviewable,
switchToBranch,
} from "../helpers/review"
import { visit } from "../helpers/page"
import { openSearch, searchFor } from "../helpers/search"
import { signUpWithWorkspace } from "../helpers/workspace"
@ -56,6 +65,98 @@ test.describe("search", () => {
await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}/)
})
test("finds text written on a draft branch", async ({ page, request }) => {
await signUpWithWorkspace(page, request)
await createDocument(page)
await documentPersisted(page)
await makeReviewable(page)
await switchToBranch(page, "draft")
await contentEditor(page).click()
await page.keyboard.type("The marmalade committee convenes")
await documentPersisted(page)
await sidebarDocument(page, "Welcome to Oxynote!").click()
await expect(page).toHaveURL(/Welcome-to-Oxynote-[a-z0-9]{20}$/)
const dialog = await openSearch(page)
const result = dialog.locator("a", { hasText: "marmalade" })
await searchFor(dialog, "marmalade", result)
// the text exists on the draft alone, so that is the branch the
// hit names and the one the link opens
await expect(result.getByTestId("search-result-branch")).toHaveText("draft")
await result.click()
await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}\?branch=[a-z0-9]{20}/)
await expect(branchSwitcher(page)).toContainText(branchLabel("draft"), {
timeout: 15_000,
})
})
test("finds a metric block by its title", async ({ page, request }) => {
await signUpWithWorkspace(page, request)
await createDocument(page)
await documentPersisted(page)
const id = documentId(page)
// a metric block has no text to type; its title is configuration,
// so the block goes in through the MCP surface the way an assistant
// would write it
const client = await connectMCPClient(
await authorizeMCPClient(page, request, "documents:read documents:write"),
)
const listing = await client.callTool({
name: "list_documents",
arguments: {},
})
const listed = JSON.parse(
(listing.content as { text: string }[])[0]?.text ?? "{}",
) as { documents: { id: string; default_branch_id: string }[] }
const branchId = listed.documents.find(
(d) => d.id === id,
)?.default_branch_id
expect(branchId).toBeDefined()
const inserted = await client.callTool({
name: "insert_block",
arguments: {
document_id: id,
branch_id: branchId,
position: "end",
// a metric lives in a grid; the root refuses a bare one
block: {
type: "metric_grid",
items: [
{ type: "metric", attrs: { title: "Pizza Fridays Scheduled" } },
],
},
},
})
expect(inserted.isError, JSON.stringify(inserted.content)).toBeFalsy()
await client.close()
// the authorization walked the page off the app, and the search
// starts from another page so that landing on the found one is
// observable in the URL
await visit(page, "/")
await sidebarDocument(page, "Welcome to Oxynote!").click()
await expect(page).toHaveURL(/Welcome-to-Oxynote-[a-z0-9]{20}$/)
const dialog = await openSearch(page)
const result = dialog.locator("a", { hasText: "Pizza Fridays" })
await searchFor(dialog, "Pizza", result)
await expect(result).toContainText("metricBlock")
const href = await result.getAttribute("href")
const uid = decodeURIComponent(href?.split("#")[1] ?? "")
expect(uid).not.toBe("")
await result.click()
await expect(page).toHaveURL(/New-Page-[a-z0-9]{20}#/)
await expect(page.locator(`[id="${uid}"]`)).toBeInViewport()
})
test("reports when nothing matches", async ({ page, request }) => {
await signUpWithWorkspace(page, request)

12
server/CLAUDE.md
View File

@ -102,7 +102,7 @@ Uploaded images live under `organizations/{org}/documents/{doc}/files/{blockUID}
- **Ordering is inverted between create and delete.** Creating writes the row before the object (upload, duplication); deleting removes the object before the row. Hooks follow the same rule from outside the transaction: `copyHooksToBranch` runs after the fork or merge commits, because `hook.NewHook` creates the watcher as a side effect and a rollback cannot take that back; a failed insert tears the just-created watcher down again. Both leave at most a row without an object, which the sweep resolves — an object without a row is invisible to every cleanup path.
- **Duplication copies, never shares.** `Document.Duplicate` regenerates every block uid, rewrites image `src` paths to the new document (swapping only the path, never the host), and returns the old→new id map the handler uses to copy each object server-side into the duplicate's own folder. Files are therefore owned by exactly one document.
- **Search jobs apply in order, per document.** A diff is a delta against the state its predecessors left, so the manager holds back every later diff of a document whose earlier one failed, rather than replaying it on top of newer work; an organization removal is global and holds everything after it. Without this a failed add replayed after a delete re-inserts the blocks of a document that no longer exists, and no later job would ever remove them.
- **Search index entries are removed from the ids the delete reports.** Only default-branch blocks are indexed, keyed by block uid with a `documentId` field. `agent.DeleteDocument` collects the subtree ids before the row goes away and returns them, and the deletion paths queue a search job carrying them through `search.Jobs` in the same transaction, so the manager clears the index by `documentId` filter — the descendants the cascade destroys have no other trace to clean up by.
- **Search index entries are branch-scoped.** Every branch of every document is indexed; an entry's id is `<branchId>-<blockUid>` (`<branchId>-docname` for the name entry), since a fork copies its source's uids, and it carries `documentId`, `branchId`, `branchName` and `branchDefault`. Every path that changes a branch queues its diff through `search.Jobs` in the same transaction: a persist on any branch, a fork (a full add of the new branch), a rename (every entry of the branch changes, since each carries the name), a merge (the target's diff). A branch deletion queues a `RemovedBranches` removal the manager applies by `branchId` filter, and a document deletion clears every branch at once: `agent.DeleteDocument` collects the subtree ids before the row goes away and returns them, so the manager clears the index by `documentId` filter — the descendants the cascade destroys have no other trace to clean up by. Index settings are declared in one object in `search.Client` and sent only when they differ from what the index reports, since a searchable or filterable attribute change re-indexes everything; every write task is awaited, so a batch Meilisearch rejects fails its job and is retried in order.
- History entries pin the files they reference, so `DB_MAX_DOCUMENT_HISTORY_ENTRIES` (count, trimmed on insert) and `DB_DOCUMENT_HISTORY_RETENTION` (age, trimmed in the sweep) also decide how long a removed image survives.
The ProseMirror schema is defined in `server/auth-realtime/src/schema/` (one file per block kind, `index.ts` aggregates). The Go-side mirror is `server/core/internal/document/node.go` (`RootBlock`, `Block`, `Mark`).
@ -201,16 +201,16 @@ onto that. Depth in those rows counts from the block itself.
**Branches are addressed by id, always.** Every content tool (`get_document`, `read_block`,
the block writes) requires `branch_id`; the default branch is a branch like any other, and
its id travels with every listing (`list_documents` and search hits carry
`default_branch_id`, `create_document` returns `branch_id`, `get_document` lists every
branch with its id). `Input.Branch`, `DocumentContent`, `DocumentBlock`, `ApplyEdit` and the
its id travels with every listing (`list_documents` carries `default_branch_id`, a search
hit carries the `branch_id` it was found on, `create_document` returns `branch_id`,
`get_document` lists every branch with its id). `Input.Branch`, `DocumentContent`, `DocumentBlock`, `ApplyEdit` and the
placement checks take the branch id and resolve it through `FetchDocumentByBranchID`,
refusing a branch that belongs to another document; an unknown id is refused with the
branches the document has (`ErrUnknownBranch`, each as "name (id)"), and a protected branch
reads but refuses every write, naming the unprotected branches to write to instead.
`Input.Document` is the default-branch fetch the document-level tools use to name and
change a document; they and search stay branch-free, and only the default branch is
indexed. No tool creates, renames, deletes or merges a branch. MCP resources name a
change a document; they stay branch-free. Search covers every branch and each hit names
its own. No tool creates, renames, deletes or merges a branch. MCP resources name a
document and a branch (`oxynote://documents/{id}/branches/{branch_id}`); the list carries
each document on its default branch. The chat client reports the document and branch it
is viewing; the session stores both unchecked, as a hint the next call verifies.

4
server/core/internal/assistant/prompt.go
View File

@ -34,7 +34,7 @@ Keep replies focused and brief: lead with the answer, keep caveats short, and us
Read tools run immediately. Write tools wait for the user's confirmation, and every write in a turn shares one confirmation, so make all the related edits in the same turn. Independent calls also go in one turn. Never invent a uid, document id or parameter value; read it first.
Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents and search hits carry each document's default_branch_id, and get_document lists every branch of a document with its id. A protected branch is read-only, so write to another branch of the document or say there is none rather than retrying.
Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents carries each document's default_branch_id, a search hit carries the branch_id it was found on, and get_document lists every branch of a document with its id. A protected branch is read-only, so write to another branch of the document or say there is none rather than retrying.
The data-source tools are read-only. Answer a question about the data from the query results; add a metric block only when the user asks for a chart, a metric or a dashboard in a document. Discover metric, label and table names with the metadata tools instead of guessing them, and run the query with the chart_type you intend before writing a metric block, so you know it renders.
@ -168,7 +168,7 @@ const _basePrompt = _personaSection + _blockModelSection + _etiquetteSection + _
// instead of waiting on a confirmation.
const _mcpIntroSection = `You are connected to Oxynote, a collaborative product for writing technical documentation. The tools operate on the documents of one organisation, which are also listed as resources. Blocks are the unit of content, addressed by uid; comments, hooks and files hang off those uids. Documents are written for humans first, balancing prose with technical detail where it sharpens meaning.
Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents and search hits carry each document's default_branch_id, and get_document lists every branch of a document with its id. Write tools apply immediately, and a protected branch is read-only, so write to another branch of the document. The data-source tools are read-only; discover metric, label and table names with the metadata tools instead of guessing them, and run a query with the chart_type you intend before writing a metric block, so you know it renders.
Read before you write: find the document, read it with get_document, and read a block in full only when you are about to edit its inner structure. Documents have branches, and every read or write of content names one by branch_id: list_documents carries each document's default_branch_id, a search hit carries the branch_id it was found on, and get_document lists every branch of a document with its id. Write tools apply immediately, and a protected branch is read-only, so write to another branch of the document. The data-source tools are read-only; discover metric, label and table names with the metadata tools instead of guessing them, and run a query with the chart_type you intend before writing a metric block, so you know it renders.
`

2
server/core/internal/assistant/tools/schema.go
View File

@ -34,7 +34,7 @@ const (
// _descBranchID describes the branch-id property every content tool
// takes.
_descBranchID = "The id of the branch to read or write: a document's default_branch_id from list_documents or a search hit, or any id from the branches get_document lists. A protected branch can be read but refuses every write."
_descBranchID = "The id of the branch to read or write: a document's default_branch_id from list_documents, a search hit's branch_id, or any id from the branches get_document lists. A protected branch can be read but refuses every write."
// _keyDocumentID is the shared document-id property name.
_keyDocumentID = "document_id"

32
server/core/internal/assistant/tools/search.go
View File

@ -3,6 +3,7 @@ package tools
import (
"fmt"
"log/slog"
"strings"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/rs/xid"
@ -46,7 +47,7 @@ type searchDocuments struct {
func (searchDocuments) Info() Info {
return Info{
Name: NameSearchDocuments,
Description: "Full-text search across the default branch of every document in the organisation for blocks whose text matches the query; other branches are not indexed. Returns hits with document_id, document_name, default_branch_id, block_uid and text; the branch id is what a follow-up get_document or read_block takes. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.",
Description: "Full-text search across every branch of every document in the organisation for blocks whose text matches the query. Returns hits with document_id, document_name, branch_id, branch_name, default (whether that branch is the document's default), block_uid and text; branch_id is what a follow-up get_document or read_block takes. A block a fork copied unchanged is found on every branch that holds it. Use it to find where a topic is discussed or which documents the user might mean; use list_documents when you know the title. A hit names the innermost block holding the text, which may sit below anything get_document lists; read_block resolves it.",
Properties: map[string]any{
_keyQuery: stringProp("The full-text search query (typo-tolerant, matches block text)."),
"limit": map[string]any{
@ -96,8 +97,8 @@ func (searchDocuments) Execute(inp Input) (string, error) {
return "", fmt.Errorf("search_documents: search: %w", err)
}
// the index stores block text keyed by (document, block uid) but not
// display names; join names in from the document tree so the AI can
// the index stores block text keyed by (branch, block uid) but not
// document names; join names in from the document tree so the AI can
// talk about hits without a follow-up lookup per document. Zero hits
// have nothing to decorate, so the fetch is skipped.
names := map[xid.ID]document.Summary{}
@ -123,11 +124,13 @@ func (searchDocuments) Execute(inp Input) (string, error) {
for _, b := range blocks {
hits = append(hits, searchHit{
DocumentID: b.DocumentID,
DocumentName: names[b.DocumentID].DocumentName,
DefaultBranchID: names[b.DocumentID].DefaultBranchID,
BlockUID: b.ID,
Text: b.Text,
DocumentID: b.DocumentID,
DocumentName: names[b.DocumentID].DocumentName,
BranchID: b.BranchID,
BranchName: b.BranchName,
Default: b.BranchDefault,
BlockUID: strings.TrimPrefix(b.ID, b.BranchID.String()+"-"),
Text: b.Text,
})
}
@ -152,10 +155,15 @@ type searchHit struct {
// and the search.
DocumentName string `json:"document_name,omitempty"`
// DefaultBranchID is the branch the hit was indexed from, which is
// the document's default one and what a follow-up read takes; zero
// when the tree lookup failed.
DefaultBranchID xid.ID `json:"default_branch_id,omitzero"`
// BranchID is the branch the hit was indexed from, which is what a
// follow-up read takes.
BranchID xid.ID `json:"branch_id"`
// BranchName is that branch's name.
BranchName string `json:"branch_name"`
// Default reports whether that branch is the document's default.
Default bool `json:"default"`
// BlockUID is the matching block's uid attribute, usable with
// read_block and the edit tools.

21
server/core/internal/assistant/tools/search_test.go
View File

@ -54,7 +54,9 @@ func Test_searchDocuments_Title(t *testing.T) {
func Test_searchDocuments_Execute(t *testing.T) {
t.Parallel()
docID := xid.New()
docID, branchID := xid.New(), xid.New()
branchFields := `"branch_id":"` + branchID.String() + `","branch_name":"draft","default":false,`
hitSearcher := func(limit *int) *SearcherMock {
return &SearcherMock{
@ -63,7 +65,14 @@ func Test_searchDocuments_Execute(t *testing.T) {
*limit = l
}
return []search.Block{{DocumentID: docID, ID: "b1", Text: "rate limiting"}}, nil
return []search.Block{{
DocumentID: docID,
BranchID: branchID,
BranchName: "draft",
BranchDefault: false,
ID: branchID.String() + "-b1",
Text: "rate limiting",
}}, nil
},
}
}
@ -120,7 +129,7 @@ func Test_searchDocuments_Execute(t *testing.T) {
Args: `{"query":"rate limit"}`,
Limit: _searchLimitDefault,
Result: `{"hits":[{"document_id":"` + docID.String() + `","document_name":"Runbook",` +
`"block_uid":"b1","text":"rate limiting"}]}`,
branchFields + `"block_uid":"b1","text":"rate limiting"}]}`,
},
"Losing the names does not fail the search": {
Searcher: hitSearcher(nil),
@ -131,7 +140,7 @@ func Test_searchDocuments_Execute(t *testing.T) {
},
Args: `{"query":"rate limit"}`,
Limit: _searchLimitDefault,
Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1","text":"rate limiting"}]}`,
Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1","text":"rate limiting"}]}`,
Logged: "cannot fetch the document tree for search hit names",
},
"Requested limit is honoured": {
@ -139,7 +148,7 @@ func Test_searchDocuments_Execute(t *testing.T) {
DB: &DBMock{},
Args: `{"query":"rate limit","limit":5}`,
Limit: 5,
Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1",` +
Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1",` +
`"text":"rate limiting"}]}`,
},
"Oversized limit is clipped": {
@ -147,7 +156,7 @@ func Test_searchDocuments_Execute(t *testing.T) {
DB: &DBMock{},
Args: `{"query":"rate limit","limit":5000}`,
Limit: _searchLimitMax,
Result: `{"hits":[{"document_id":"` + docID.String() + `","block_uid":"b1",` +
Result: `{"hits":[{"document_id":"` + docID.String() + `",` + branchFields + `"block_uid":"b1",` +
`"text":"rate limiting"}]}`,
},
}

2
server/core/internal/assistant/tools/testdata/tool_schemas.golden vendored
View File

File diff suppressed because one or more lines are too long

21
server/core/internal/document/document.go
View File

@ -262,21 +262,22 @@ func (d Document) HistoryEntry() HistoryEntry {
}
}
// Search converts the document to a search.Document for indexing.
// Search converts the document's branch to search blocks for indexing.
func (d Document) Search() map[string]search.Block {
res := d.Content.Search(d.OrganizationID, d.ID)
res[d.ID.String()] = search.Block{
// a special block carrying the document name. The map is keyed by the
// document ID while the indexed block ID is prefixed, so the title
// never collides with a content block that happens to share the ID.
ID: "docname" + d.ID.String(),
scope := search.Scope{
OrganizationID: d.OrganizationID,
DocumentID: d.ID,
Type: "document",
Text: d.DocumentName,
BranchID: d.BranchID,
BranchName: d.BranchName,
BranchDefault: d.Default,
}
res := d.Content.Search(scope)
// a special block carrying the document name. The map is keyed by the
// document ID, which no content block shares.
res[d.ID.String()] = scope.Block("docname", "document", d.DocumentName)
return res
}

7
server/core/internal/document/document_test.go
View File

@ -270,15 +270,20 @@ func Test_Document_Search(t *testing.T) {
nameBlock, ok := res[doc.ID.String()]
require.True(t, ok)
assert.Equal(t, search.Block{
ID: "docname" + doc.ID.String(),
ID: doc.BranchID.String() + "-docname",
OrganizationID: "org-1",
DocumentID: doc.ID,
BranchID: doc.BranchID,
BranchName: DefaultBranch,
BranchDefault: true,
Type: "document",
Text: "Runbook",
}, nameBlock)
contentBlock, ok := res["p1"]
require.True(t, ok)
assert.Equal(t, doc.BranchID.String()+"-p1", contentBlock.ID)
assert.Equal(t, doc.BranchID, contentBlock.BranchID)
assert.Equal(t, "hello", contentBlock.Text)
}

23
server/core/internal/document/node.go
View File

@ -70,11 +70,11 @@ func (rb RootBlock) HasBlock(blockID string) bool {
}
// Search transforms RootBlock into a search compatible type.
func (rb RootBlock) Search(organizationID string, documentID xid.ID) map[string]search.Block {
func (rb RootBlock) Search(scope search.Scope) map[string]search.Block {
res := make(map[string]search.Block)
for _, b := range rb.Content {
content := b.Search(organizationID, documentID)
content := b.Search(scope)
maps.Insert(res, maps.All(content))
}
@ -210,7 +210,7 @@ func (b Block) HasBlock(blockID string) bool {
}
// Search transforms Block into a search compatible type.
func (b Block) Search(organizationID string, documentID xid.ID) map[string]search.Block {
func (b Block) Search(scope search.Scope) map[string]search.Block {
res := make(map[string]search.Block)
var text strings.Builder
@ -224,20 +224,21 @@ func (b Block) Search(organizationID string, documentID xid.ID) map[string]searc
continue
}
content := cb.Search(organizationID, documentID)
content := cb.Search(scope)
maps.Insert(res, maps.All(content))
}
// a metric block has no text children; its title is an attribute.
if b.Type == BlockNodeMetricBlock {
if title, ok := b.Attrs[AttrTitle].(string); ok {
text.WriteString(title)
}
}
if text.Len() != 0 {
if id, ok := b.UID(); ok && id != "" {
res[id] = search.Block{
ID: id,
OrganizationID: organizationID,
DocumentID: documentID,
Type: string(b.Type),
Text: text.String(),
}
res[id] = scope.Block(id, string(b.Type), text.String())
}
}

48
server/core/internal/document/node_test.go
View File

@ -296,33 +296,37 @@ func Test_RootBlock_Search(t *testing.T) {
},
// no text: nothing to index.
{Type: BlockNodeHorizontalRule, Attrs: Attributes{"uid": "hr1"}},
// a metric block's title is an attribute, not a text child.
{
Type: BlockNodeMetricBlock,
Attrs: Attributes{"uid": "m1", "title": "Pizza Fridays"},
},
// an untitled metric block has nothing to index.
{
Type: BlockNodeMetricBlock,
Attrs: Attributes{"uid": "m2", "title": ""},
},
{
Type: BlockNodeMetricBlock,
Attrs: Attributes{"uid": "m3"},
},
},
}
res := rb.Search("org-1", documentID)
scope := search.Scope{
OrganizationID: "org-1",
DocumentID: documentID,
BranchID: xid.New(),
BranchName: "draft",
}
res := rb.Search(scope)
assert.Equal(t, map[string]search.Block{
"p1": {
ID: "p1",
OrganizationID: "org-1",
DocumentID: documentID,
Type: "paragraph",
Text: "first",
},
"li1": {
ID: "li1",
OrganizationID: "org-1",
DocumentID: documentID,
Type: "listItem",
Text: "nested",
},
"p2": {
ID: "p2",
OrganizationID: "org-1",
DocumentID: documentID,
Type: "paragraph",
Text: "plain bold tail",
},
"p1": scope.Block("p1", "paragraph", "first"),
"li1": scope.Block("li1", "listItem", "nested"),
"p2": scope.Block("p2", "paragraph", "plain bold tail"),
"m1": scope.Block("m1", "metricBlock", "Pizza Fridays"),
}, res)
}

217
server/core/internal/search/client.go
View File

@ -7,7 +7,9 @@ import (
"encoding/json"
"errors"
"fmt"
"maps"
"net/http"
"slices"
"strconv"
"strings"
"time"
@ -41,8 +43,37 @@ const (
// _taskPollInterval is how often an asynchronous Meilisearch task is
// polled while waiting for it to settle.
_taskPollInterval = 50 * time.Millisecond
// _setupTimeout bounds the index setup at boot.
_setupTimeout = time.Minute
// _writeTimeout bounds one batch of index writes, task waits included.
_writeTimeout = 2 * time.Minute
// _searchTimeout bounds one search request.
_searchTimeout = 10 * time.Second
)
// _displayedAttributes are the entry fields search hits carry.
var _displayedAttributes = []string{
"id",
"organizationId",
"documentId",
"branchId",
"branchName",
"branchDefault",
"type",
_textAttribute,
}
// _filterableAttributes are the entry fields searches and removals filter
// by.
var _filterableAttributes = []string{
"organizationId",
"documentId",
"branchId",
}
// Client is a Meilisearch client wrapper.
type Client struct {
meiliMan meilisearch.ServiceManager
@ -76,8 +107,12 @@ func (c *Client) Configured() bool {
return c.meiliMan != nil
}
// ensureIndex ensures the documents index exists.
// ensureIndex ensures the documents index exists and runs with the
// declared settings.
func (c *Client) ensureIndex(ctx context.Context) error {
ctx, cancel := context.WithTimeout(ctx, _setupTimeout)
defer cancel()
_, err := c.meiliMan.GetIndexWithContext(ctx, _documentsIndex)
switch merr, ok := errors.AsType[*meilisearch.Error](err); {
@ -99,30 +134,70 @@ func (c *Client) ensureIndex(ctx context.Context) error {
return fmt.Errorf("getting documents index: %w", err)
}
// the settings updates are idempotent and are therefore applied on every
// boot. Applying them only when the index is created leaves an index
// whose settings task failed broken for good — with organizationId not
// filterable every search fails — and never rolls new synonyms out to an
// existing deployment.
task, err := c.meiliMan.Index(_documentsIndex).UpdateFilterableAttributesWithContext(ctx, &[]any{
"organizationId",
"documentId",
})
// the settings are checked on every boot rather than only when the
// index is created: an index whose settings task failed would otherwise
// stay broken for good, and new synonyms would never reach an existing
// deployment. They are sent only when they differ, since a searchable
// or filterable attribute change re-indexes every entry.
want, err := indexSettings()
if err != nil {
return fmt.Errorf("updating filterable attributes: %w", err)
return err
}
have, err := c.meiliMan.Index(_documentsIndex).GetSettingsWithContext(ctx)
if err != nil {
return fmt.Errorf("getting settings: %w", err)
}
if settingsMatch(have, want) {
return nil
}
task, err := c.meiliMan.Index(_documentsIndex).UpdateSettingsWithContext(ctx, want)
if err != nil {
return fmt.Errorf("updating settings: %w", err)
}
if err = c.awaitTask(ctx, task); err != nil {
return fmt.Errorf("updating filterable attributes: %w", err)
}
if err = c.setupSynonyms(ctx, _documentsIndex); err != nil {
return fmt.Errorf("setting up synonyms: %w", err)
return fmt.Errorf("updating settings: %w", err)
}
return nil
}
// indexSettings returns the settings the documents index runs with. Only
// the fields it sets are sent; everything else keeps Meilisearch's default.
func indexSettings() (*meilisearch.Settings, error) {
var synonyms map[string][]string
if err := json.Unmarshal(synonymsFile, &synonyms); err != nil {
return nil, fmt.Errorf("unmarshaling synonyms: %w", err)
}
return &meilisearch.Settings{
SearchableAttributes: []string{_textAttribute},
DisplayedAttributes: _displayedAttributes,
FilterableAttributes: _filterableAttributes,
Synonyms: synonyms,
}, nil
}
// settingsMatch reports whether the index already runs with every field
// want sets. Attribute lists are compared as sets, since Meilisearch does
// not promise their order back.
func settingsMatch(have, want *meilisearch.Settings) bool {
sameSet := func(a, b []string) bool {
a, b = slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b))
return slices.Equal(a, b)
}
return sameSet(have.SearchableAttributes, want.SearchableAttributes) &&
sameSet(have.DisplayedAttributes, want.DisplayedAttributes) &&
sameSet(have.FilterableAttributes, want.FilterableAttributes) &&
maps.EqualFunc(have.Synonyms, want.Synonyms, sameSet)
}
// awaitTask blocks until an asynchronous Meilisearch task settles and reports
// a rejected task as an error, so a failed index or settings update surfaces
// at boot instead of leaving the index quietly misconfigured.
@ -133,27 +208,28 @@ func (c *Client) awaitTask(ctx context.Context, info *meilisearch.TaskInfo) erro
}
if task.Status != meilisearch.TaskStatusSucceeded {
return fmt.Errorf("task %d finished with status %q", info.TaskUID, task.Status)
return fmt.Errorf(
"task %d finished with status %q: %s (%s)",
info.TaskUID,
task.Status,
task.Error.Message,
task.Error.Code,
)
}
return nil
}
// setupSynonyms sets up synonyms for the documents index.
func (c *Client) setupSynonyms(ctx context.Context, index string) error {
var synonyms map[string][]string
if err := json.Unmarshal(synonymsFile, &synonyms); err != nil {
return fmt.Errorf("unmarshaling synonyms: %w", err)
}
task, err := c.meiliMan.Index(index).UpdateSynonymsWithContext(ctx, &synonyms)
// settle reports the outcome of one index write: the request error when the
// call itself failed, otherwise the task's result once Meilisearch has
// applied it.
func (c *Client) settle(ctx context.Context, what string, task *meilisearch.TaskInfo, err error) error {
if err != nil {
return fmt.Errorf("updating synonyms: %w", err)
return fmt.Errorf("%s: %w", what, err)
}
if err = c.awaitTask(ctx, task); err != nil {
return fmt.Errorf("updating synonyms: %w", err)
if err := c.awaitTask(ctx, task); err != nil {
return fmt.Errorf("%s: %w", what, err)
}
return nil
@ -165,6 +241,9 @@ func (c *Client) SearchDocuments(ctx context.Context, organizationID, query stri
return nil, ErrNotConfigured
}
ctx, cancel := context.WithTimeout(ctx, _searchTimeout)
defer cancel()
res, err := c.meiliMan.Index(_documentsIndex).SearchWithContext(ctx, query, &meilisearch.SearchRequest{
AttributesToSearchOn: []string{_textAttribute},
AttributesToCrop: []string{_textAttribute},
@ -206,6 +285,9 @@ func (c *Client) SearchDocumentBlocks(ctx context.Context, organizationID, query
return nil, ErrNotConfigured
}
ctx, cancel := context.WithTimeout(ctx, _searchTimeout)
defer cancel()
res, err := c.meiliMan.Index(_documentsIndex).SearchWithContext(ctx, query, &meilisearch.SearchRequest{
AttributesToSearchOn: []string{_textAttribute},
Filter: fmt.Sprintf("organizationId = %q", organizationID),
@ -231,23 +313,25 @@ func (c *Client) SearchDocumentBlocks(ctx context.Context, organizationID, query
}
// ReplaceDocumentBlocks replaces document blocks in the index based on the
// provided differences.
// provided differences. Every write is awaited, so a batch Meilisearch
// rejects is reported rather than silently dropped.
func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference) error {
if !c.Configured() {
return ErrNotConfigured
}
if len(bd.Added) != 0 {
_, err := c.meiliMan.Index(_documentsIndex).AddDocumentsWithContext(ctx, bd.Added, nil)
if err != nil {
return fmt.Errorf("adding documents: %w", err)
}
}
ctx, cancel := context.WithTimeout(ctx, _writeTimeout)
defer cancel()
if len(bd.Updated) != 0 {
_, err := c.meiliMan.Index(_documentsIndex).UpdateDocumentsWithContext(ctx, bd.Updated, nil)
if err != nil {
return fmt.Errorf("updating documents: %w", err)
idx := c.meiliMan.Index(_documentsIndex)
// every entry is complete, so added and updated ones alike are sent as
// one add-or-replace request.
if docs := slices.Concat(bd.Added, bd.Updated); len(docs) != 0 {
task, err := idx.AddDocumentsWithContext(ctx, docs, nil)
if err = c.settle(ctx, "adding documents", task, err); err != nil {
return err
}
}
@ -258,9 +342,22 @@ func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference)
ids = append(ids, block.ID)
}
_, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsWithContext(ctx, ids, nil)
if err != nil {
return fmt.Errorf("deleting documents: %w", err)
task, err := idx.DeleteDocumentsWithContext(ctx, ids, nil)
if err = c.settle(ctx, "deleting documents", task, err); err != nil {
return err
}
}
if len(bd.RemovedBranches) != 0 {
ids := make([]string, 0, len(bd.RemovedBranches))
for _, br := range bd.RemovedBranches {
ids = append(ids, br.BranchID.String())
}
if err := c.deleteByFilter(ctx, idx, "branchId", ids); err != nil {
return fmt.Errorf("deleting branches by filter: %w", err)
}
}
@ -268,31 +365,35 @@ func (c *Client) ReplaceDocumentBlocks(ctx context.Context, bd BlocksDifference)
ids := make([]string, 0, len(bd.RemovedDocuments))
for _, id := range bd.RemovedDocuments {
ids = append(ids, strconv.Quote(id.String()))
ids = append(ids, id.String())
}
filter := fmt.Sprintf("documentId IN [%s]", strings.Join(ids, ", "))
_, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsByFilterWithContext(ctx, filter, nil)
if err != nil {
if err := c.deleteByFilter(ctx, idx, "documentId", ids); err != nil {
return fmt.Errorf("deleting documents by filter: %w", err)
}
}
if len(bd.RemovedOrganizations) != 0 {
ids := make([]string, 0, len(bd.RemovedOrganizations))
for _, id := range bd.RemovedOrganizations {
ids = append(ids, strconv.Quote(id))
}
filter := fmt.Sprintf("organizationId IN [%s]", strings.Join(ids, ", "))
_, err := c.meiliMan.Index(_documentsIndex).DeleteDocumentsByFilterWithContext(ctx, filter, nil)
if err != nil {
if err := c.deleteByFilter(ctx, idx, "organizationId", bd.RemovedOrganizations); err != nil {
return fmt.Errorf("deleting organizations by filter: %w", err)
}
}
return nil
}
// deleteByFilter removes every entry whose field is one of the ids and
// waits for the removal to apply.
func (c *Client) deleteByFilter(ctx context.Context, idx meilisearch.IndexManager, field string, ids []string) error {
quoted := make([]string, 0, len(ids))
for _, id := range ids {
quoted = append(quoted, strconv.Quote(id))
}
filter := fmt.Sprintf("%s IN [%s]", field, strings.Join(quoted, ", "))
task, err := idx.DeleteDocumentsByFilterWithContext(ctx, filter, nil)
return c.settle(ctx, "deleting by "+field, task, err)
}

254
server/core/internal/search/client_test.go
View File

@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"slices"
"testing"
"time"
@ -86,70 +87,116 @@ func Test_NewClient(t *testing.T) {
}
}
wasFilterableUpdated := func(count int) check {
wasSettingsUpdated := func(count int) check {
return func(t *testing.T, _ *mock.MeiliServiceManager, idx *mock.MeiliIndexManager, _ error) {
ff := idx.UpdateFilterableAttributesWithContextCalls()
ff := idx.UpdateSettingsWithContextCalls()
require.Len(t, ff, count)
if count == 0 {
return
}
assert.Equal(t, &[]any{"organizationId", "documentId"}, ff[0].Request)
assert.Equal(t, []string{"text"}, ff[0].Request.SearchableAttributes)
assert.Equal(t, _displayedAttributes, ff[0].Request.DisplayedAttributes)
assert.Equal(t, _filterableAttributes, ff[0].Request.FilterableAttributes)
assert.NotEmpty(t, ff[0].Request.Synonyms)
}
}
wasSynonymsUpdated := func(count int) check {
return func(t *testing.T, _ *mock.MeiliServiceManager, idx *mock.MeiliIndexManager, _ error) {
ff := idx.UpdateSynonymsWithContextCalls()
require.Len(t, ff, count)
// current is what a settled index reports: the declared settings, with
// the attribute lists in an order of Meilisearch's choosing.
current := func() *meilisearch.Settings {
want, err := indexSettings()
require.NoError(t, err)
if count == 0 {
return
}
have := *want
have.DisplayedAttributes = slices.Clone(want.DisplayedAttributes)
slices.Reverse(have.DisplayedAttributes)
require.NotNil(t, ff[0].Request)
assert.NotEmpty(t, *ff[0].Request)
return &have
}
// fresh is what a just-created index reports.
fresh := func() *meilisearch.Settings {
return &meilisearch.Settings{
SearchableAttributes: []string{"*"},
DisplayedAttributes: []string{"*"},
}
}
tests := map[string]struct {
GetIndexErr error
CreateErr error
FilterableErr error
SynonymsErr error
WaitErr error
TaskStatus meilisearch.TaskStatus
Checks []check
GetIndexErr error
CreateErr error
Settings *meilisearch.Settings
GetSettingsErr error
SettingsErr error
WaitErr error
TaskStatus meilisearch.TaskStatus
Checks []check
}{
"Existing index still has its settings applied": {
"Existing index with the declared settings is left alone": {
Settings: current(),
Checks: checks(
hasError(false),
wasCreateIndexCalled(0),
wasFilterableUpdated(1),
wasSynonymsUpdated(1),
wasSettingsUpdated(0),
),
},
"Existing index with stale settings gets them updated": {
Settings: func() *meilisearch.Settings {
st := current()
st.FilterableAttributes = []string{"organizationId", "documentId"}
return st
}(),
Checks: checks(
hasError(false),
wasCreateIndexCalled(0),
wasSettingsUpdated(1),
),
},
"Existing index with stale synonyms gets them updated": {
Settings: func() *meilisearch.Settings {
st := current()
st.Synonyms = map[string][]string{"2fa": {"mfa"}}
return st
}(),
Checks: checks(
hasError(false),
wasSettingsUpdated(1),
),
},
"Settings fetch failure fails": {
GetSettingsErr: assert.AnError,
Checks: checks(
hasError(true),
wasSettingsUpdated(0),
),
},
"Rejected settings task fails": {
Settings: fresh(),
TaskStatus: meilisearch.TaskStatusFailed,
Checks: checks(
hasError(true),
wasCreateIndexCalled(0),
wasSettingsUpdated(1),
),
},
"Task wait failure fails": {
WaitErr: assert.AnError,
Settings: fresh(),
WaitErr: assert.AnError,
Checks: checks(
hasError(true),
),
},
"Missing index is created with settings": {
GetIndexErr: indexNotFoundErr(),
Settings: fresh(),
Checks: checks(
hasError(false),
wasCreateIndexCalled(1),
wasFilterableUpdated(1),
wasSynonymsUpdated(1),
wasSettingsUpdated(1),
),
},
"Other meilisearch errors fail": {
@ -171,22 +218,16 @@ func Test_NewClient(t *testing.T) {
CreateErr: assert.AnError,
Checks: checks(
hasError(true),
wasFilterableUpdated(0),
wasSettingsUpdated(0),
),
},
"Filterable attribute failure fails": {
GetIndexErr: indexNotFoundErr(),
FilterableErr: assert.AnError,
Checks: checks(
hasError(true),
wasSynonymsUpdated(0),
),
},
"Synonyms failure fails": {
"Settings update failure fails": {
GetIndexErr: indexNotFoundErr(),
SynonymsErr: assert.AnError,
Settings: fresh(),
SettingsErr: assert.AnError,
Checks: checks(
hasError(true),
wasSettingsUpdated(1),
),
},
}
@ -196,11 +237,11 @@ func Test_NewClient(t *testing.T) {
t.Parallel()
idx := &mock.MeiliIndexManager{
UpdateFilterableAttributesWithContextFunc: func(context.Context, *[]any) (*meilisearch.TaskInfo, error) {
return &meilisearch.TaskInfo{TaskUID: 2}, tc.FilterableErr
GetSettingsWithContextFunc: func(context.Context) (*meilisearch.Settings, error) {
return tc.Settings, tc.GetSettingsErr
},
UpdateSynonymsWithContextFunc: func(context.Context, *map[string][]string) (*meilisearch.TaskInfo, error) {
return &meilisearch.TaskInfo{TaskUID: 3}, tc.SynonymsErr
UpdateSettingsWithContextFunc: func(context.Context, *meilisearch.Settings) (*meilisearch.TaskInfo, error) {
return &meilisearch.TaskInfo{TaskUID: 2}, tc.SettingsErr
},
}
@ -234,9 +275,19 @@ func Test_NewClient(t *testing.T) {
}
// newTestClient builds a Client wired straight to the given index mock,
// bypassing ensureIndex.
func newTestClient(idx *mock.MeiliIndexManager) *Client {
return &Client{meiliMan: stubManagers(idx)}
// bypassing ensureIndex. Every task the index reports settles with the
// given status.
func newTestClient(idx *mock.MeiliIndexManager, status meilisearch.TaskStatus) *Client {
svc := stubManagers(idx)
svc.WaitForTaskWithContextFunc = func(context.Context, int64, time.Duration) (*meilisearch.Task, error) {
task := &meilisearch.Task{Status: status}
task.Error.Message = "boom"
task.Error.Code = "invalid_document_id"
return task, nil
}
return &Client{meiliMan: svc}
}
func Test_Client_SearchDocuments(t *testing.T) {
@ -259,7 +310,7 @@ func Test_Client_SearchDocuments(t *testing.T) {
},
}
data, err := newTestClient(idx).SearchDocuments(context.Background(), "org-1", "hello")
data, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocuments(context.Background(), "org-1", "hello")
require.NoError(t, err)
assert.JSONEq(t, `[{"id": "b1", "text": "<mark>hello</mark>"}]`, string(data))
@ -281,7 +332,7 @@ func Test_Client_SearchDocuments(t *testing.T) {
},
}
_, err := newTestClient(idx).SearchDocuments(context.Background(), "org-1", "hello")
_, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocuments(context.Background(), "org-1", "hello")
require.Error(t, err)
})
}
@ -314,7 +365,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) {
},
}
blocks, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
blocks, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
require.NoError(t, err)
assert.Equal(t, []Block{{
@ -343,7 +394,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) {
},
}
_, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
_, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
require.Error(t, err)
})
@ -356,7 +407,7 @@ func Test_Client_SearchDocumentBlocks(t *testing.T) {
},
}
_, err := newTestClient(idx).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
_, err := newTestClient(idx, meilisearch.TaskStatusSucceeded).SearchDocumentBlocks(context.Background(), "org-1", "hello", 7)
require.Error(t, err)
})
}
@ -385,12 +436,6 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
}
}
wasUpdateCalled := func(count int) check {
return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
require.Len(t, idx.UpdateDocumentsWithContextCalls(), count)
}
}
wasDeleteCalled := func(count int) check {
return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
require.Len(t, idx.DeleteDocumentsWithContextCalls(), count)
@ -403,7 +448,16 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
}
}
hasFilter := func(filter string) check {
return func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
ff := idx.DeleteDocumentsByFilterWithContextCalls()
require.NotEmpty(t, ff)
assert.Equal(t, filter, ff[0].Filter)
}
}
docID1, docID2 := xid.New(), xid.New()
branchID1, branchID2 := xid.New(), xid.New()
fullDiff := BlocksDifference{
Added: []Block{{ID: "a"}},
@ -413,8 +467,8 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
tests := map[string]struct {
Diff BlocksDifference
TaskStatus meilisearch.TaskStatus
AddErr error
UpdateErr error
DeleteErr error
DeleteFilterErr error
Checks []check
@ -423,26 +477,29 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
Checks: checks(
hasError(false),
wasAddCalled(0),
wasUpdateCalled(0),
wasDeleteCalled(0),
wasDeleteByFilterCalled(0),
),
},
"Removed branches are cleared by filter": {
Diff: BlocksDifference{RemovedBranches: []BranchRemoval{
{DocumentID: docID1, BranchID: branchID1},
{DocumentID: docID1, BranchID: branchID2},
}},
Checks: checks(
hasError(false),
wasDeleteCalled(0),
wasDeleteByFilterCalled(1),
hasFilter(fmt.Sprintf("branchId IN [%q, %q]", branchID1, branchID2)),
),
},
"Removed documents are cleared by filter": {
Diff: BlocksDifference{RemovedDocuments: []xid.ID{docID1, docID2}},
Checks: checks(
hasError(false),
wasDeleteCalled(0),
wasDeleteByFilterCalled(1),
func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
ff := idx.DeleteDocumentsByFilterWithContextCalls()
require.NotEmpty(t, ff)
assert.Equal(
t,
fmt.Sprintf("documentId IN [%q, %q]", docID1, docID2),
ff[0].Filter,
)
},
hasFilter(fmt.Sprintf("documentId IN [%q, %q]", docID1, docID2)),
),
},
"Removed organizations are cleared by filter": {
@ -451,11 +508,7 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
hasError(false),
wasDeleteCalled(0),
wasDeleteByFilterCalled(1),
func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
ff := idx.DeleteDocumentsByFilterWithContextCalls()
require.NotEmpty(t, ff)
assert.Equal(t, `organizationId IN ["org-1", "org-2"]`, ff[0].Filter)
},
hasFilter(`organizationId IN ["org-1", "org-2"]`),
),
},
"Delete by filter failure is propagated": {
@ -466,17 +519,17 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
wasDeleteByFilterCalled(1),
),
},
"Full difference dispatches all three": {
// added and updated entries are complete either way, so one
// add-or-replace request carries both.
"Full difference sends one add and one delete": {
Diff: fullDiff,
Checks: checks(
hasError(false),
wasAddCalled(1),
wasUpdateCalled(1),
wasDeleteCalled(1),
func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
ff := idx.DeleteDocumentsWithContextCalls()
require.NotEmpty(t, ff)
assert.Equal(t, []string{"r1", "r2"}, ff[0].Identifiers)
assert.Equal(t, []Block{{ID: "a"}, {ID: "u"}}, idx.AddDocumentsWithContextCalls()[0].DocumentsPtr)
assert.Equal(t, []string{"r1", "r2"}, idx.DeleteDocumentsWithContextCalls()[0].Identifiers)
},
),
},
@ -485,16 +538,6 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
AddErr: assert.AnError,
Checks: checks(
hasError(true),
wasUpdateCalled(0),
wasDeleteCalled(0),
),
},
"Update failure short-circuits": {
Diff: fullDiff,
UpdateErr: assert.AnError,
Checks: checks(
hasError(true),
wasAddCalled(1),
wasDeleteCalled(0),
),
},
@ -504,7 +547,30 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
Checks: checks(
hasError(true),
wasAddCalled(1),
wasUpdateCalled(1),
),
},
// the request is accepted but the task fails later; the batch is
// reported so the job is retried rather than dropped.
"Failed task is reported with its cause": {
Diff: fullDiff,
TaskStatus: meilisearch.TaskStatusFailed,
Checks: checks(
hasError(true),
wasAddCalled(1),
wasDeleteCalled(0),
func(t *testing.T, _ *mock.MeiliIndexManager, err error) {
assert.ErrorContains(t, err, "boom (invalid_document_id)")
},
),
},
"Calls run under a deadline": {
Diff: fullDiff,
Checks: checks(
hasError(false),
func(t *testing.T, idx *mock.MeiliIndexManager, _ error) {
_, ok := idx.AddDocumentsWithContextCalls()[0].Ctx.Deadline()
assert.True(t, ok)
},
),
},
}
@ -515,20 +581,22 @@ func Test_Client_ReplaceDocumentBlocks(t *testing.T) {
idx := &mock.MeiliIndexManager{
AddDocumentsWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) {
return nil, tc.AddErr
},
UpdateDocumentsWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) {
return nil, tc.UpdateErr
return &meilisearch.TaskInfo{TaskUID: 1}, tc.AddErr
},
DeleteDocumentsWithContextFunc: func(context.Context, []string, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) {
return nil, tc.DeleteErr
return &meilisearch.TaskInfo{TaskUID: 2}, tc.DeleteErr
},
DeleteDocumentsByFilterWithContextFunc: func(context.Context, any, *meilisearch.DocumentOptions) (*meilisearch.TaskInfo, error) {
return nil, tc.DeleteFilterErr
return &meilisearch.TaskInfo{TaskUID: 3}, tc.DeleteFilterErr
},
}
err := newTestClient(idx).ReplaceDocumentBlocks(context.Background(), tc.Diff)
status := meilisearch.TaskStatusSucceeded
if tc.TaskStatus != "" {
status = tc.TaskStatus
}
err := newTestClient(idx, status).ReplaceDocumentBlocks(context.Background(), tc.Diff)
for _, ch := range tc.Checks {
ch(t, idx, err)

58
server/core/internal/search/document.go
View File

@ -21,6 +21,15 @@ type Block struct {
// DocumentID is the identifier for the document this block belongs to.
DocumentID xid.ID `json:"documentId"`
// BranchID is the identifier for the branch this block belongs to.
BranchID xid.ID `json:"branchId"`
// BranchName is the name of the branch this block belongs to.
BranchName string `json:"branchName"`
// BranchDefault reports whether the branch is the document's default.
BranchDefault bool `json:"branchDefault"`
// Type is the type of the block (e.g., "paragraph", "heading").
Type string `json:"type"`
@ -28,6 +37,50 @@ type Block struct {
Text string `json:"text"`
}
// Scope is the branch every block of one indexing pass belongs to.
type Scope struct {
// OrganizationID is the organization owning the document.
OrganizationID string
// DocumentID is the document the branch belongs to.
DocumentID xid.ID
// BranchID is the branch the blocks are read from.
BranchID xid.ID
// BranchName is the branch's name.
BranchName string
// BranchDefault reports whether the branch is the document's default.
BranchDefault bool
}
// Block builds an index entry for uid within the scope. The entry id is
// prefixed with the branch id: a fork copies its source's content, uids
// included, so the uid alone names one block on every branch at once.
func (s Scope) Block(uid, typ, text string) Block {
return Block{
ID: s.BranchID.String() + "-" + uid,
OrganizationID: s.OrganizationID,
DocumentID: s.DocumentID,
BranchID: s.BranchID,
BranchName: s.BranchName,
BranchDefault: s.BranchDefault,
Type: typ,
Text: text,
}
}
// BranchRemoval names a branch whose every block is removed. The document
// id is carried so the removal is ordered with the document's other jobs.
type BranchRemoval struct {
// DocumentID is the document the branch belonged to.
DocumentID xid.ID `json:"documentId"`
// BranchID is the removed branch.
BranchID xid.ID `json:"branchId"`
}
// BlocksDifference represents the differences between two slices of Blocks.
type BlocksDifference struct {
// Updated contains blocks that have been modified.
@ -39,6 +92,11 @@ type BlocksDifference struct {
// Removed contains blocks that have been removed.
Removed []Block `json:"removed"`
// RemovedBranches contains branches whose every block is removed.
// Deleting a branch drops its content row, so the index is cleared by
// branch instead of by block.
RemovedBranches []BranchRemoval `json:"removedBranches"`
// RemovedDocuments contains documents whose every block is removed.
// Deleting a document cascades to its descendants in the database,
// which makes enumerating their blocks impossible after the fact, so

24
server/core/internal/search/document_test.go
View File

@ -4,6 +4,7 @@ import (
"encoding/json"
"testing"
"github.com/rs/xid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -79,3 +80,26 @@ func Test_BlocksDifference_Scan(t *testing.T) {
assert.Error(t, decoded.Scan("not bytes"))
assert.Error(t, decoded.Scan([]byte(`{not json`)))
}
func Test_Scope_Block(t *testing.T) {
t.Parallel()
scope := Scope{
OrganizationID: "org-1",
DocumentID: xid.New(),
BranchID: xid.New(),
BranchName: "draft",
BranchDefault: false,
}
assert.Equal(t, Block{
ID: scope.BranchID.String() + "-p1",
OrganizationID: "org-1",
DocumentID: scope.DocumentID,
BranchID: scope.BranchID,
BranchName: "draft",
BranchDefault: false,
Type: "paragraph",
Text: "hello",
}, scope.Block("p1", "paragraph", "hello"))
}

4
server/core/internal/search/manager/manager.go
View File

@ -145,6 +145,10 @@ func jobScope(bd search.BlocksDifference) (documentIDs []xid.ID, global bool) {
}
}
for _, br := range bd.RemovedBranches {
add(br.DocumentID)
}
for _, id := range bd.RemovedDocuments {
add(id)
}

28
server/core/internal/search/manager/manager_test.go
View File

@ -212,6 +212,34 @@ func Test_Manager_processJobs(t *testing.T) {
},
),
},
"A branch removal waits behind its document's failed diff": {
Batches: [][]search.DocumentSearchJob{{
docJob(1, docA),
{
ID: 2,
BlockDiff: search.BlocksDifference{
RemovedBranches: []search.BranchRemoval{{DocumentID: docA, BranchID: xid.New()}},
},
},
{
ID: 3,
BlockDiff: search.BlocksDifference{
RemovedBranches: []search.BranchRemoval{{DocumentID: docB, BranchID: xid.New()}},
},
},
}},
FailJobs: map[int64]bool{1: true},
Checks: checks(
hasError(false),
wasReplaceCalled(2),
wasDeleteCalled(1),
func(t *testing.T, db *DBMock, _ *SearchGatewayMock, _ error) {
ff := db.DeleteDocumentSearchJobCalls()
require.Len(t, ff, 1)
assert.Equal(t, int64(3), ff[0].ID, "the other document's branch removal keeps moving")
},
),
},
"An organization removal waits for anything held": {
Batches: [][]search.DocumentSearchJob{{
docJob(1, docA),

99
server/core/internal/server/internal/document/branch_http_test.go
View File

@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/guregu/null/v5"
@ -147,6 +148,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Metadata int
Maintainers int
SearchJobs int
BranchID xid.ID
}{
"Missing branch ID parameter": {
DB: &DBMock{},
@ -273,7 +275,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Metadata: 1,
SearchJobs: 1,
},
"Non-default branch skips the search job": {
"Non-default branch queues its own search job": {
DB: &DBMock{
FetchDocumentUnsafeByBranchIDFunc: func(context.Context, xid.ID) (*documentCore.Document, error) {
return branchDoc(_branchID2), nil
@ -284,10 +286,12 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
return []string{"u1", "u9"}, nil
},
},
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
SearchJobs: 1,
BranchID: _branchID2,
},
}
@ -306,6 +310,18 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
assert.Equal(t, c.Metadata, cnt.metadata)
assert.Equal(t, c.Maintainers, cnt.maintainers)
assert.Len(t, c.Tx.InsertDocumentSearchJobCalls(), c.SearchJobs)
// the diff is scoped to the persisted branch: entries are keyed
// by its id, whichever branch it is.
if !c.BranchID.IsZero() {
diff := c.Tx.InsertDocumentSearchJobCalls()[0].Diff
require.NotEmpty(t, diff.Updated)
for _, b := range diff.Updated {
assert.Equal(t, c.BranchID, b.BranchID)
assert.True(t, strings.HasPrefix(b.ID, c.BranchID.String()+"-"))
}
}
})
}
}
@ -1188,6 +1204,23 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
RespCode: http.StatusInternalServerError,
Committed: 1,
},
"Search job insert error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return storedDoc(), nil
},
},
Tx: &TxMock{
FetchDocumentFunc: func(context.Context, xid.ID, string, string) (*documentCore.Document, error) {
return branchDoc(_branchID2), nil
},
InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Successful branch creation": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
@ -1227,6 +1260,17 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
if c.RespCode == http.StatusCreated {
require.Len(t, c.Tx.ForkDocumentBranchCalls(), 1)
assert.Equal(t, "feature", c.Tx.ForkDocumentBranchCalls()[0].TargetBranch)
// the fork's entries are added under its own branch id before
// the commit, so the branch is searchable without an edit.
require.Len(t, c.Tx.InsertDocumentSearchJobCalls(), 1)
diff := c.Tx.InsertDocumentSearchJobCalls()[0].Diff
require.NotEmpty(t, diff.Added)
for _, b := range diff.Added {
assert.Equal(t, _branchID2, b.BranchID)
assert.True(t, strings.HasPrefix(b.ID, _branchID2.String()+"-"))
}
}
if len(c.CopiedHooks) == 0 {
@ -1244,12 +1288,14 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
func Test_Handler_DeleteDocumentBranch(t *testing.T) {
cc := map[string]struct {
DB *DBMock
Tx *TxMock
NoSession bool
OmitDoc bool
OmitBranch bool
RespCode int
RespJSON string
Deleted int
Committed int
}{
"No session in context": {
DB: &DBMock{},
@ -1326,6 +1372,8 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) {
CountDocumentBranchesFunc: func(context.Context, xid.ID, string) (int, error) {
return 2, nil
},
},
Tx: &TxMock{
DeleteDocumentBranchByIDFunc: func(context.Context, xid.ID, string) error {
return errors.New("boom")
},
@ -1333,6 +1381,23 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) {
RespCode: http.StatusInternalServerError,
Deleted: 1,
},
"Search job insert error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return branchDoc(_branchID), nil
},
CountDocumentBranchesFunc: func(context.Context, xid.ID, string) (int, error) {
return 2, nil
},
},
Tx: &TxMock{
InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error {
return errors.New("boom")
},
},
RespCode: http.StatusInternalServerError,
Deleted: 1,
},
"Successful deletion": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
@ -1342,8 +1407,9 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) {
return 2, nil
},
},
RespCode: http.StatusNoContent,
Deleted: 1,
RespCode: http.StatusNoContent,
Deleted: 1,
Committed: 1,
},
}
@ -1351,7 +1417,12 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl, _ := newTestHandler(c.DB, &fakePublisher{})
tx := c.Tx
if tx == nil {
tx = &TxMock{}
}
hdl, _ := newTestHandler(withTx(c.DB, tx, nil), &fakePublisher{})
rec := httptest.NewRecorder()
@ -1363,7 +1434,17 @@ func Test_Handler_DeleteDocumentBranch(t *testing.T) {
assert.JSONEq(t, c.RespJSON, rec.Body.String())
}
assert.Len(t, c.DB.DeleteDocumentBranchByIDCalls(), c.Deleted)
assert.Len(t, tx.DeleteDocumentBranchByIDCalls(), c.Deleted)
assert.Len(t, tx.CommitCalls(), c.Committed)
// the branch row is gone with its content, so the index is
// cleared by branch id.
if c.RespCode == http.StatusNoContent {
require.Len(t, tx.InsertDocumentSearchJobCalls(), 1)
assert.Equal(t, search.BlocksDifference{
RemovedBranches: []search.BranchRemoval{{DocumentID: _documentID, BranchID: _branchID}},
}, tx.InsertDocumentSearchJobCalls()[0].Diff)
}
})
}
}

68
server/core/internal/server/internal/document/http.go
View File

@ -617,7 +617,29 @@ func (h *Handler) UpdateDocumentBranch(w http.ResponseWriter, r *http.Request) {
ndoc := doc.ApplyBranchUpdate(ui.Name, ui.Protected, session.UserID)
if err := h.db.UpdateDocumentBranchMetadata(r.Context(), ndoc); err != nil {
var tx Tx
if err := h.db.BeginTx(r.Context(), &tx); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
defer tx.Rollback() //nolint:errcheck // error provides no meaningful info
if err := tx.UpdateDocumentBranchMetadata(r.Context(), ndoc); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
// the index carries the branch name on every entry of the branch.
if ndoc.BranchName != doc.BranchName {
if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDiff(doc.Search(), ndoc.Search())); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
}
if err := tx.Commit(); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
@ -755,15 +777,13 @@ func (h *Handler) UpdateDocumentBranchByIDUnsafe(w http.ResponseWriter, r *http.
}
}
if ndoc.BranchName == documentCore.DefaultBranch {
if err = h.searchJobs.Enqueue(
r.Context(),
tx,
search.BlocksDiff(doc.Search(), ndoc.Search()),
); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err = h.searchJobs.Enqueue(
r.Context(),
tx,
search.BlocksDiff(doc.Search(), ndoc.Search()),
); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
err = tx.Commit()
@ -1258,6 +1278,11 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) {
return
}
if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDiff(nil, newDoc.Search())); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err := tx.Commit(); err != nil {
httpserver.RespondError(h.log, w, err)
return
@ -1333,7 +1358,28 @@ func (h *Handler) DeleteDocumentBranch(w http.ResponseWriter, r *http.Request) {
return
}
if err := h.db.DeleteDocumentBranchByID(r.Context(), branchID, session.ActiveOrganizationID); err != nil {
var tx Tx
if err := h.db.BeginTx(r.Context(), &tx); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
defer tx.Rollback() //nolint:errcheck // error provides no meaningful info
if err := tx.DeleteDocumentBranchByID(r.Context(), branchID, session.ActiveOrganizationID); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err := h.searchJobs.Enqueue(r.Context(), tx, search.BlocksDifference{
RemovedBranches: []search.BranchRemoval{{DocumentID: branchDoc.ID, BranchID: branchID}},
}); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err := tx.Commit(); err != nil {
httpserver.RespondError(h.log, w, err)
return
}

43
server/core/internal/server/internal/document/http_test.go
View File

@ -1395,6 +1395,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
cc := map[string]struct {
DB *DBMock
Tx *TxMock
NoSession bool
OmitDoc bool
Body string
@ -1402,6 +1403,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
RespCode int
Updated int
Metadata int
Jobs int
}{
"No session in context": {
DB: &DBMock{},
@ -1450,6 +1452,8 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return branchDoc(_branchID), nil
},
},
Tx: &TxMock{
UpdateDocumentBranchMetadataFunc: func(context.Context, documentCore.Document) error {
return errors.New("boom")
},
@ -1458,6 +1462,22 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
RespCode: http.StatusInternalServerError,
Updated: 1,
},
"Search job insert error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return branchDoc(_branchID), nil
},
},
Tx: &TxMock{
InsertDocumentSearchJobFunc: func(context.Context, search.BlocksDifference) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
Updated: 1,
Jobs: 1,
},
// the main branch is found by its flag in the tree and content
// queries, but its name is what the user sees; renaming it would
// leave the document looking nameless.
@ -1493,6 +1513,7 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
RespCode: http.StatusOK,
Updated: 1,
Metadata: 1,
Jobs: 1,
},
}
@ -1500,21 +1521,37 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl, cnt := newTestHandler(c.DB, &fakePublisher{})
tx := c.Tx
if tx == nil {
tx = &TxMock{}
}
hdl, cnt := newTestHandler(withTx(c.DB, tx, nil), &fakePublisher{})
rec := httptest.NewRecorder()
hdl.UpdateDocumentBranch(rec, newRequest(http.MethodPut, c.Body, c.NoSession, c.OmitDoc, false))
assert.Equal(t, c.RespCode, rec.Code)
assert.Len(t, c.DB.UpdateDocumentBranchMetadataCalls(), c.Updated)
assert.Len(t, tx.UpdateDocumentBranchMetadataCalls(), c.Updated)
assert.Len(t, tx.InsertDocumentSearchJobCalls(), c.Jobs)
assert.Equal(t, c.Metadata, cnt.metadata)
if c.RespCode == http.StatusOK {
ff := c.DB.UpdateDocumentBranchMetadataCalls()
ff := tx.UpdateDocumentBranchMetadataCalls()
require.NotEmpty(t, ff)
assert.Equal(t, c.WantName, ff[0].Doc.BranchName)
assert.True(t, ff[0].Doc.Protected)
assert.Len(t, tx.CommitCalls(), 1)
}
// a rename rewrites every entry of the branch, since each one
// carries the branch name.
if c.Jobs == 1 && c.RespCode == http.StatusOK {
diff := tx.InsertDocumentSearchJobCalls()[0].Diff
require.Len(t, diff.Updated, 1)
assert.Equal(t, _branchID.String()+"-docname", diff.Updated[0].ID)
assert.Equal(t, "Renamed", diff.Updated[0].BranchName)
}
})
}

94
web/app/components/SearchModal.nuxt.test.ts
View File

@ -12,6 +12,23 @@ import { clearTeleportedOverlays, seedAuthOrganization } from "./test-helpers"
const DEBOUNCE_MS = 300
const DOC_ID = "doc1".padEnd(20, "0")
const MAIN_ID = "main".padEnd(20, "0")
const DRAFT_ID = "draft".padEnd(20, "0")
// hit builds a search hit on the main branch, the way core indexes one:
// the entry id carries the branch prefix
function hit(uid: string, type: string, text: string) {
return {
id: `${MAIN_ID}-${uid}`,
documentId: DOC_ID,
organizationId: "org1",
branchId: MAIN_ID,
branchName: "main",
branchDefault: true,
type,
text,
}
}
function seedTree() {
seedQueryData(
@ -130,9 +147,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
})
it("lists the matching documents", async ({ expect }) => {
mockSearch([
{ id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" },
])
mockSearch([hit("docname", "document", "Runbook")])
await mountModal()
await search("run")
@ -145,9 +160,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
}) => {
seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" })
seedTree()
mockSearch([
{ id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" },
])
mockSearch([hit("docname", "document", "Runbook")])
await mountModal()
await search("run")
@ -162,14 +175,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
}) => {
seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" })
seedTree()
mockSearch([
{
id: "block-7",
documentId: DOC_ID,
type: "heading",
text: "Rollback steps",
},
])
mockSearch([hit("block-7", "heading", "Rollback steps")])
await mountModal()
await search("roll")
@ -179,13 +185,57 @@ describe("<SearchModal>", { concurrent: false }, () => {
)
})
it("links a hit on another branch with that branch selected", async ({
expect,
}) => {
seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" })
seedTree()
mockSearch([
{
...hit("block-7", "heading", "Rollback steps"),
id: `${DRAFT_ID}-block-7`,
branchId: DRAFT_ID,
branchName: "draft",
branchDefault: false,
},
])
await mountModal()
await search("roll")
expect(document.body.querySelector("a")?.getAttribute("href")).toBe(
`/Acme-Corp/Runbook-${DOC_ID}?branch=${DRAFT_ID}#block-7`,
)
})
it("names the branch of every hit", async ({ expect }) => {
seedTree()
mockSearch([
hit("docname", "document", "Runbook"),
{
...hit("block-7", "heading", "Rollback steps"),
id: `${DRAFT_ID}-block-7`,
branchId: DRAFT_ID,
branchName: "draft",
branchDefault: false,
},
])
await mountModal()
await search("r")
const branches = [
...document.body.querySelectorAll("[data-testid='search-result-branch']"),
].map((el) => el.textContent.trim())
expect(branches).toEqual(["main", "draft"])
})
it("falls back to the bare document id when the tree has no such document", async ({
expect,
}) => {
seedAuthOrganization({ name: "Acme Corp", slug: "acme-corp" })
mockSearch([
{ id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" },
])
mockSearch([hit("docname", "document", "Runbook")])
await mountModal()
await search("run")
@ -199,9 +249,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
expect,
}) => {
seedTree()
mockSearch([
{ id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" },
])
mockSearch([hit("docname", "document", "Runbook")])
await mountModal()
await search("run")
@ -218,7 +266,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
])(
"marks a $type hit with its own icon",
async ({ type, expected }, { expect }) => {
mockSearch([{ id: "x", documentId: DOC_ID, type: type, text: "Hit" }])
mockSearch([hit("x", type, "Hit")])
await mountModal()
await search("hit")
@ -242,9 +290,7 @@ describe("<SearchModal>", { concurrent: false }, () => {
it("closes when a result is followed", async ({ expect }) => {
seedTree()
mockSearch([
{ id: DOC_ID, documentId: DOC_ID, type: "document", text: "Runbook" },
])
mockSearch([hit("docname", "document", "Runbook")])
const wrapper = await mountModal()
await search("run")

31
web/app/components/SearchModal.vue
View File

@ -82,13 +82,28 @@ function resultLink(result: DocumentSearchResult) {
let href = orgSlug ? `/${orgSlug}/${docSlug}` : `/${docSlug}`
// the document page opens the default branch unless told otherwise
if (!result.branchDefault) {
href = href + `?branch=${encodeURIComponent(result.branchId)}`
}
if (result.type !== "document") {
href = href + `#${encodeURIComponent(result.id)}`
href = href + `#${encodeURIComponent(blockUid(result))}`
}
return href
}
// blockUid strips the branch prefix off an index entry id, leaving the
// block's uid, which is what the page anchors on
function blockUid(result: DocumentSearchResult) {
const prefix = `${result.branchId}-`
return result.id.startsWith(prefix)
? result.id.slice(prefix.length)
: result.id
}
function resultIcon(type: string) {
switch (type) {
case "document":
@ -171,7 +186,7 @@ function resultTypeText(type: string) {
<div v-else class="flex w-full flex-col gap-2">
<NuxtLink
v-for="result in searchResults"
:key="`${result.documentId}-${result.id}`"
:key="result.id"
:href="resultLink(result)"
:prefetch="false"
class="flex w-full gap-1 rounded-md py-1 no-underline transition hover:bg-accent/50 active:bg-accent"
@ -188,8 +203,16 @@ function resultTypeText(type: string) {
v-html="sanitizeSearchResult(result.text)"
/>
<!-- eslint-enable vue/no-v-html -->
<div class="text-2sm text-muted-foreground capitalize">
{{ resultTypeText(result.type) }}
<div class="text-2sm text-muted-foreground">
<span class="capitalize">
{{ resultTypeText(result.type) }}
</span>
<span
data-testid="search-result-branch"
class="before:px-1 before:content-['·']"
>
{{ result.branchName }}
</span>
</div>
</div>
</NuxtLink>

59
web/app/pages/[[organizationSlug]]/[[documentSlug]].vue
View File

@ -90,10 +90,14 @@ definePageMeta({
(!docSlugName || !equalNameSlugs(docSlugName, docRealName)))
) {
return navigateTo(
`/${createNameSlug(orgRealName)}/${createNameSlugWithId(
docRealName,
docSlugInfo.id,
)}${to.hash || ""}`,
{
path: `/${createNameSlug(orgRealName)}/${createNameSlugWithId(
docRealName,
docSlugInfo.id,
)}`,
query: to.query,
hash: to.hash,
},
{
replace: true,
},
@ -223,6 +227,33 @@ watchImmediate(
},
)
// a ?branch=<id> query opens that branch of the document. It keys on the
// route rather than the store so a later switch through the header is not
// undone; an id the document does not have is ignored and the watch below
// falls back to the default branch
watchImmediate(
[() => pageRoute.query.branch, fetchBranches.state],
([queryBranch, branchState]) => {
const branches = branchState.data
if (typeof queryBranch !== "string" || !branches?.length) {
return
}
const branch = branches.find((b) => b.branchId === queryBranch)
const defaultBranch = branches.find((b) => b.default)
if (!branch || branch.branchId === editorStore.activeBranchId) {
return
}
editorStore.updateActiveBranchId(branch.branchId)
editorStore.updateTargetBranchId(
branch.default ? null : (defaultBranch?.branchId ?? null),
)
},
)
watchImmediate(
[
fetchBranches.state,
@ -371,12 +402,14 @@ function applyDocumentNameChange(name: string) {
protected: activeDocMetadata.value.protected,
})
void pageRouter.replace(
`/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId(
void pageRouter.replace({
path: `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId(
name,
activeDocMetadata.value.id,
)}${pageRoute.hash || ""}`,
)
)}`,
query: pageRoute.query,
hash: pageRoute.hash,
})
}
function refreshOrganizationRouteSlug() {
@ -384,12 +417,14 @@ function refreshOrganizationRouteSlug() {
return
}
void pageRouter.replace(
`/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId(
void pageRouter.replace({
path: `/${createNameSlug(fetchOrganization.data.value?.data?.slug || "")}/${createNameSlugWithId(
activeDocMetadata.value.name,
activeDocMetadata.value.id,
)}${pageRoute.hash || ""}`,
)
)}`,
query: pageRoute.query,
hash: pageRoute.hash,
})
}
function applyIconChange(icon: string) {

5
web/app/utils/api/document.ts
View File

@ -225,9 +225,12 @@ export function defaultDocumentHookState(
}
export interface DocumentSearchResult {
id: string // block/node ID
id: string // branch-scoped index entry ID
documentId: string
organizationId: string
branchId: string
branchName: string
branchDefault: boolean
type: string
text: string
}