diff --git a/README.md b/README.md index baddf13..27c7092 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,32 @@ yarn --cwd packages/backend add @rwdocs/backstage-plugin-rw-backend yarn --cwd packages/backend add @rwdocs/backstage-plugin-search-backend-module-rw ``` +## RW 0.1.36 compatibility + +The viewer requires Node `^22.22.2 || >=24.15.0`. Update the RW backend, +search collator, and frontend dependencies together to pick up the core and +viewer fixes in [RW 0.1.36](https://github.com/rwdocs/rw/releases/tag/v0.1.36). + +For S3, deploy upgraded **backend and search collator readers before publishing +bundles that declare `name`**. Older readers accept these bundles but ignore the +explicit name; upgraded readers can still read existing bundles. + +RW metadata can now declare `name` alongside `kind` to choose a section identity +independently of its URL and title. This does not create catalog entities: align +catalog annotations and references with the intended section identity. Changing +an existing identity creates no aliases and does not migrate comments. + +Homepage names and kinds are resolved from the site's navigation; they do not +need to use the implicit `section:/root` spelling. Root links prefer a +matching catalog entity exposing the same site's root. Otherwise they fall back +to the source entity's Docs tab, which must expose the whole site, either unscoped +or scoped to the actual homepage root. + +Rename legacy RW frontmatter and sidecar `type` fields to `kind`; `type` is now +ignored there. Unrelated Backstage configuration fields named `type` are unchanged. +See [RW page metadata](https://github.com/rwdocs/rw/blob/v0.1.36/docs/metadata.md) +for naming rules and migration details. + ## Backend Setup Add the plugin to your backend in `packages/backend/src/index.ts`: @@ -102,3 +128,11 @@ The backend persists its SQLite database (catalog, search index, …) under a `.data/` directory at the repo root, so state survives restarts instead of being rebuilt from scratch each time. The directory is git-ignored. Delete it (`rm -rf .data`) for a clean database. + +## Duplicate section identities + +Full section refs should be unique. For collisions, registry and search follow +RW's canonical section-root lookup and log a warning. Pages whose identity +resolves to another path are omitted from these indexes; independently named +child sections can remain indexed. Direct site-path reads are unchanged. Fix the +conflicting metadata to restore omitted pages to identity-based surfaces. diff --git a/plugins/rw-backend/package.json b/plugins/rw-backend/package.json index 9efb7d0..fd1016b 100644 --- a/plugins/rw-backend/package.json +++ b/plugins/rw-backend/package.json @@ -61,7 +61,7 @@ "@backstage/types": "^1.2.2", "@rwdocs/backstage-plugin-rw-common": "workspace:^", "@rwdocs/backstage-plugin-rw-node": "workspace:^", - "@rwdocs/core": "^0.1.35", + "@rwdocs/core": "^0.1.36", "express": "^4.21.0", "express-promise-router": "^4.1.0", "luxon": "^3.7.2", diff --git a/plugins/rw-backend/src/router.core.test.ts b/plugins/rw-backend/src/router.core.test.ts new file mode 100644 index 0000000..2d79f17 --- /dev/null +++ b/plugins/rw-backend/src/router.core.test.ts @@ -0,0 +1,144 @@ +import * as http from "http"; +import { mkdtemp, mkdir, writeFile, rm } from "fs/promises"; +import { tmpdir } from "os"; +import { join } from "path"; +import { mockServices } from "@backstage/backend-test-utils"; +import type { RwSite } from "@rwdocs/core"; +import express from "express"; +import request from "supertest"; +import { Hub } from "./hub"; +import { createRouter } from "./router"; +import { SiteAuthorizer } from "./authorizeSite"; + +const sectionRef = "system:commerce/payments-api"; +const sectionMarkdown = [ + "---", + "kind: system", + "namespace: commerce", + "name: payments-api", + "title: Payments documentation", + "---", + "# Payments", + "", +].join("\n"); +const runbookMarkdown = "# Operations\n\nRestart the payment worker.\n"; + +describe("createRouter with real core", () => { + let projectDir: string | undefined; + let site: RwSite; + let server: http.Server; + + beforeAll(async () => { + projectDir = await mkdtemp(join(tmpdir(), "rw-router-core-")); + await mkdir(join(projectDir, "docs/payments-guide"), { recursive: true }); + await writeFile(join(projectDir, "rw.toml"), ""); + await writeFile( + join(projectDir, "docs/index.md"), + "---\nkind: section\nnamespace: commerce\nname: handbook\n---\n# Home\n", + ); + await writeFile(join(projectDir, "docs/payments-guide/index.md"), sectionMarkdown); + await writeFile(join(projectDir, "docs/payments-guide/runbook.md"), runbookMarkdown); + + const hub = new Hub({ projectDir, entity: "component:default/test" }); + site = hub.getSite("default/component/test")!; + const httpAuth = mockServices.httpAuth.mock(); + const authorizer = new SiteAuthorizer({ + permissions: mockServices.permissions.mock(), + httpAuth, + auditor: mockServices.auditor.mock(), + }); + jest.spyOn(authorizer, "assertReadable").mockResolvedValue(undefined); + const router = await createRouter({ + logger: mockServices.logger.mock(), + httpAuth, + hub, + authorizer, + }); + const app = express().use(router); + server = http.createServer(app); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + server.off("error", reject); + resolve(); + }); + }); + }); + + afterAll(async () => { + try { + if (server?.listening) { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + } finally { + try { + if (projectDir) await rm(projectDir, { recursive: true, force: true }); + } finally { + jest.restoreAllMocks(); + } + } + }); + + it("serves the named homepage identity in unscoped navigation", async () => { + const response = await request(server).get("/site/default/component/test/navigation"); + expect(response.status).toBe(200); + expect(response.body.scope).toMatchObject({ + path: "/", + section: { kind: "section", namespace: "commerce", name: "handbook" }, + }); + }); + + it("lists and resolves explicit identities independently of page paths", async () => { + expect(await site.listSections()).toEqual( + expect.arrayContaining([expect.objectContaining({ sectionRef, path: "payments-guide" })]), + ); + const pages = await site.listPages(); + expect(pages).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + sectionRef, + subpath: "", + path: "payments-guide", + title: "Payments documentation", + }), + expect.objectContaining({ + sectionRef, + subpath: "runbook", + path: "payments-guide/runbook", + anchors: expect.arrayContaining([{ sectionRef, subpath: "runbook" }]), + }), + ]), + ); + await expect(site.pagePathFor(sectionRef, "")).resolves.toBe("payments-guide"); + await expect(site.pagePathFor(sectionRef, "runbook")).resolves.toBe("payments-guide/runbook"); + }); + + it("serves the scoped section root at its unchanged page path", async () => { + const response = await request(server) + .get("/site/default/component/test/pages/") + .query({ sectionRef }); + expect(response.status).toBe(200); + expect(response.body.meta).toMatchObject({ + sectionRef, + subpath: "", + path: "/payments-guide", + title: "Payments documentation", + }); + }); + + it.each([ + ["section root", undefined, sectionMarkdown], + ["nested page", "runbook", runbookMarkdown], + ] as const)( + "reads %s Markdown using the explicit identity", + async (_label, subpath, markdown) => { + const response = await request(server) + .get("/site/default/component/test/markdown") + .query(subpath === undefined ? { sectionRef } : { sectionRef, subpath }); + expect(response.status).toBe(200); + expect(response.body).toEqual({ markdown }); + }, + ); +}); diff --git a/plugins/rw-backend/src/siteIndex/runWorker.test.ts b/plugins/rw-backend/src/siteIndex/runWorker.test.ts index 573849a..fa77172 100644 --- a/plugins/rw-backend/src/siteIndex/runWorker.test.ts +++ b/plugins/rw-backend/src/siteIndex/runWorker.test.ts @@ -8,7 +8,18 @@ import { runWorker } from "./runWorker"; function fakeSite() { return { listSections: async () => [{ sectionRef: "component:default/docs", path: "", ancestors: [] }], - listPages: async () => [{ sectionRef: "component:default/docs", subpath: "", title: "Home" }], + listPages: async () => [ + { + sectionRef: "component:default/docs", + subpath: "", + path: "", + anchors: [{ sectionRef: "component:default/docs", subpath: "" }], + hasContent: true, + title: "Home", + lastModified: "2026-06-24T00:00:00Z", + }, + ], + pagePathFor: async () => "", }; } @@ -28,11 +39,189 @@ const deps = (knex: Knex, makeSite: any, now?: () => Date) => ({ rng: () => 0.5, }); +const root = "section:default/root"; +const shared = "system:default/shared"; +const independent = "component:default/independent"; +const deep = "component:default/deep"; +const siteRef = "component:default/docs"; + +function collisionSite(nested = false) { + const sections = [ + { sectionRef: root, path: "", ancestors: [] }, + { sectionRef: shared, path: "a", ancestors: [root] }, + { sectionRef: shared, path: "a-b", ancestors: [root] }, + { sectionRef: independent, path: "a-b/unique", ancestors: [shared, root] }, + ...(nested + ? [ + { sectionRef: shared, path: "a/nested", ancestors: [shared, root] }, + { sectionRef: deep, path: "a/nested/deep", ancestors: [shared, shared, root] }, + ] + : []), + ]; + function page(sectionRef: string, subpath: string, path: string, title: string) { + return { + sectionRef, + subpath, + path, + title, + hasContent: true, + lastModified: "2026-06-24T00:00:00Z", + anchors: sections + .filter((s) => s.path === "" || path === s.path || path.startsWith(`${s.path}/`)) + .sort((a, b) => b.path.length - a.path.length) + .map((s) => ({ + sectionRef: s.sectionRef, + subpath: s.path ? path.slice(s.path.length).replace(/^\//, "") : path, + })), + }; + } + const pages = [ + page(root, "", "", "Home"), + page(shared, "", "a", "Winner"), + page(shared, "", "a-b", "Loser"), + page(shared, "q", "a/q", "Winner q"), + page(shared, "q", "a-b/q", "Loser q"), + page(shared, "only", "a-b/only", "Loser only"), + page(independent, "p", "a-b/unique/p", "Independent p"), + ...(nested ? [page(deep, "p", "a/nested/deep/p", "Deep p")] : []), + ]; + return { + sections, + pages, + listSections: async () => sections, + listPages: async () => pages, + pagePathFor: jest.fn(async (ref: string) => + ref === shared ? "a" : (sections.find((s) => s.sectionRef === ref)?.path ?? null), + ), + }; +} + describe("runWorker", () => { let knex: Knex; beforeEach(async () => (knex = await createTestDb())); afterEach(async () => knex.destroy()); + it.each([false, true])( + "commits canonical duplicate identities and stable ownership (nested=%s)", + async (nested) => { + const site = collisionSite(nested); + const sectionOwnershipStore = new SectionOwnershipStore(knex); + await sectionOwnershipStore.swapSite(siteRef, [ + { + site_ref: siteRef, + section_ref: shared, + entity_ref: shared, + entity_owner_ref: "group:default/shared-owners", + }, + { + site_ref: siteRef, + section_ref: siteRef, + entity_ref: siteRef, + entity_owner_ref: "group:default/host-owners", + }, + ]); + await new SiteRefreshStore(knex).upsertSite(siteRef, new Date("2026-06-24T00:00:00Z")); + const workerDeps = { + ...deps( + knex, + () => site, + () => new Date("2026-06-24T00:01:00Z"), + ), + sectionOwnershipStore, + logger: { warn: jest.fn(), info: jest.fn(), debug: jest.fn() } as any, + }; + await runWorker(workerDeps); + const titles = await knex("pages").pluck("title"); + expect(titles).toEqual( + expect.arrayContaining(["Home", "Winner", "Winner q", "Independent p"]), + ); + expect(titles.sort()).toEqual( + ["Home", "Winner", "Winner q", "Independent p", ...(nested ? ["Deep p"] : [])].sort(), + ); + expect(await knex("pages").where({ title: "Loser" })).toHaveLength(0); + const refresh = await knex("site_refresh").where({ site_ref: siteRef }).first(); + expect(refresh.last_built_at).not.toBeNull(); + expect(refresh.result_hash).not.toBeNull(); + expect(await knex("sections").where({ section_ref: independent }).first()).toMatchObject({ + entity_ref: siteRef, + parent_section_ref: root, + section_path: "a-b/unique", + }); + expect(await knex("sections").where({ section_ref: shared }).first()).toMatchObject({ + entity_ref: shared, + parent_section_ref: root, + section_path: "", + }); + const deepRows = await knex("sections") + .where({ section_ref: deep }) + .select("entity_ref", "parent_section_ref", "section_path"); + expect(deepRows).toEqual( + nested + ? [ + { + entity_ref: shared, + parent_section_ref: shared, + section_path: "nested/deep", + }, + ] + : [], + ); + expect(workerDeps.logger.warn).toHaveBeenCalledTimes(1); + expect(workerDeps.logger.warn).toHaveBeenCalledWith(expect.stringContaining(siteRef), { + siteRef, + collidingRefs: 1, + omittedSections: nested ? 2 : 1, + omittedPages: 3, + }); + + await knex("site_refresh").update({ next_update_at: new Date("2026-06-24T00:00:00Z") }); + site.sections.reverse(); + site.pages.reverse(); + await expect(site.pagePathFor(shared)).resolves.toBe("a"); + const swap = jest.spyOn(workerDeps.registryStore, "swapSite"); + await runWorker({ ...workerDeps, now: () => new Date("2026-06-25T00:00:00Z") }); + const rebuilt = await knex("site_refresh").where({ site_ref: siteRef }).first(); + expect(rebuilt.result_hash).toBe(refresh.result_hash); + expect(rebuilt.last_built_at).not.toBeNull(); + expect(rebuilt.last_built_at).not.toEqual(refresh.last_built_at); + expect(swap).not.toHaveBeenCalled(); + }, + ); + + it.each(["null", "rejected"])( + "keeps the prior registry and records a failed collision lookup (%s)", + async (failure) => { + const workerDeps = deps(knex, fakeSite, () => new Date("2026-06-24T00:01:00Z")); + await workerDeps.siteRefreshStore.upsertSite(siteRef, new Date("2026-06-24T00:00:00Z")); + await runWorker(workerDeps); + const before = await knex("site_refresh").where({ site_ref: siteRef }).first(); + expect(before.result_hash).not.toBeNull(); + const sectionsBefore = await knex("sections").select("*"); + const pagesBefore = await knex("pages").select("*"); + await knex("site_refresh").update({ next_update_at: new Date("2026-06-24T00:00:00Z") }); + const site = collisionSite(); + if (failure === "null") site.pagePathFor.mockResolvedValue(null); + else site.pagePathFor.mockRejectedValue(new Error("lookup failed")); + const swap = jest.spyOn(workerDeps.registryStore, "swapSite"); + await runWorker({ + ...workerDeps, + makeSite: () => site, + now: () => new Date("2026-06-25T00:00:00Z"), + }); + const after = await knex("site_refresh").where({ site_ref: siteRef }).first(); + expect(after.errors).toContain( + failure === "null" + ? `Cannot resolve canonical section root for ${shared}` + : "lookup failed", + ); + expect(after.result_hash).toBe(before.result_hash); + expect(after.last_built_at).toEqual(before.last_built_at); + expect(swap).not.toHaveBeenCalled(); + expect(await knex("sections").select("*")).toEqual(sectionsBefore); + expect(await knex("pages").select("*")).toEqual(pagesBefore); + }, + ); + it("builds a due site: writes sections/pages and marks built", async () => { const store = new SiteRefreshStore(knex); await store.upsertSite("component:default/docs", new Date("2026-06-24T00:00:00Z")); @@ -59,6 +248,7 @@ describe("runWorker", () => { throw new Error("s3 down"); }, listPages: async () => [], + pagePathFor: async () => null, }); await runWorker(deps(knex, makeSite)); const row = await knex("site_refresh").where({ site_ref: "component:default/docs" }).first(); @@ -102,14 +292,23 @@ describe("runWorker", () => { // another where they come back reversed. The effective ownership rows differ // in insertion order but the resulting hash stored in site_refresh must be // identical — proving that runWorker sorts effective before hashing. - const siteRef = "component:default/docs"; const sectionsForward = [ { sectionRef: "component:default/a", path: "a", ancestors: [] }, { sectionRef: "component:default/b", path: "b", ancestors: [] }, ]; const sectionsReversed = [...sectionsForward].reverse(); - const pages = [{ sectionRef: "component:default/a", subpath: "", title: "Home" }]; + const pages = [ + { + sectionRef: "component:default/a", + subpath: "", + path: "a", + anchors: [{ sectionRef: "component:default/a", subpath: "" }], + hasContent: true, + title: "Home", + lastModified: "2026-06-24T00:00:00Z", + }, + ]; const claims = [ { @@ -138,6 +337,7 @@ describe("runWorker", () => { () => ({ listSections: async () => listSectionsResult, listPages: async () => pages, + pagePathFor: async () => "a", }), () => new Date("2026-06-24T00:01:00Z"), ), @@ -153,11 +353,12 @@ describe("runWorker", () => { const hashForward = await buildAndGetHash(sectionsForward); const hashReversed = await buildAndGetHash(sectionsReversed); + expect(hashForward).not.toBeNull(); + expect(hashReversed).not.toBeNull(); expect(hashForward).toBe(hashReversed); }); it("passes section rows carrying effective ownership to swapSite", async () => { - const siteRef = "component:default/docs"; const store = new SiteRefreshStore(knex); await store.upsertSite(siteRef, new Date("2026-06-24T00:00:00Z")); diff --git a/plugins/rw-backend/src/siteIndex/runWorker.ts b/plugins/rw-backend/src/siteIndex/runWorker.ts index 0cb03e9..ef61e59 100644 --- a/plugins/rw-backend/src/siteIndex/runWorker.ts +++ b/plugins/rw-backend/src/siteIndex/runWorker.ts @@ -1,6 +1,6 @@ import pLimit from "p-limit"; import type { LoggerService } from "@backstage/backend-plugin-api"; -import { toEntityPath } from "@rwdocs/backstage-plugin-rw-common"; +import { projectSiteListing, toEntityPath } from "@rwdocs/backstage-plugin-rw-common"; import type { RwSite } from "@rwdocs/core"; import type { SiteRefreshStore } from "./SiteRefreshStore"; import type { RegistryStore } from "./RegistryStore"; @@ -25,7 +25,7 @@ export async function runWorker(deps: { siteRefreshStore: SiteRefreshStore; registryStore: RegistryStore; sectionOwnershipStore: Pick; - makeSite: (entityPath: string) => Pick; + makeSite: (entityPath: string) => Pick; now?: () => Date; rng?: () => number; }): Promise { @@ -53,12 +53,21 @@ export async function runWorker(deps: { site.listPages(), sectionOwnershipStore.listForSite(siteRef), ]); + const projected = await projectSiteListing(rawSections, rawPages, (ref) => + site.pagePathFor(ref, ""), + ); + if (projected.diagnostics.collidingRefs > 0) { + logger.warn(`Canonical section identity collisions in site ${siteRef}`, { + siteRef, + ...projected.diagnostics, + }); + } // computeSectionRows folds the effective-ownership rollup into each dense section row. // registryHash is order-sensitive (JSON.stringify) and listSections order is unspecified, // so sort by section_ref for a stable hash. - const sections = sortSections(computeSectionRows(siteRef, rawSections, claims)); + const sections = sortSections(computeSectionRows(siteRef, projected.sections, claims)); const pages = sortPages( - rawPages.map((p) => ({ + projected.pages.map((p) => ({ site_ref: siteRef, section_ref: p.sectionRef, subpath: p.subpath, diff --git a/plugins/rw-backend/src/siteIndex/siteListing.core.test.ts b/plugins/rw-backend/src/siteIndex/siteListing.core.test.ts new file mode 100644 index 0000000..171826e --- /dev/null +++ b/plugins/rw-backend/src/siteIndex/siteListing.core.test.ts @@ -0,0 +1,129 @@ +import { mkdtemp, mkdir, writeFile, rm } from "fs/promises"; +import { tmpdir } from "os"; +import { join } from "path"; +import { createSite } from "@rwdocs/core"; +import { projectSiteListing } from "@rwdocs/backstage-plugin-rw-common"; + +const root = "section:default/root"; +const shared = "system:default/shared"; +const independent = "component:default/independent"; +const deep = "component:default/deep"; +const ordering = "system:default/ordering"; +const unicode = "system:default/unicode"; +const winnerMarkdown = "# Winner q\n\nCanonical winner body.\n"; + +describe("site listing projection with real core", () => { + it("roundtrips retained identities to canonical Markdown and physical ancestry", async () => { + const projectDir = await mkdtemp(join(tmpdir(), "rw-site-listing-core-")); + try { + const write = async (path: string, markdown: string) => { + const target = join(projectDir, "docs", path); + await mkdir(join(target, ".."), { recursive: true }); + await writeFile(target, markdown); + }; + const section = async (path: string, kind: string, name: string, title: string) => { + await write( + path ? `${path}/index.md` : "index.md", + `---\nkind: ${kind}\nname: ${name}\ntitle: ${title}\n---\n# ${title}\n`, + ); + }; + await writeFile(join(projectDir, "rw.toml"), ""); + await section("", "section", "root", "Home"); + await section("a", "system", "shared", "Winner"); + await section("a-b", "system", "shared", "Loser"); + await section("a/nested", "system", "shared", "Nested loser"); + await section("a-b/unique", "component", "independent", "Independent"); + await section("a/nested/deep", "component", "deep", "Deep"); + await write("a/q.md", winnerMarkdown); + await write("a-b/q.md", "# Loser q\n\nWrong body.\n"); + await write("a-b/only.md", "# Loser only\n"); + await write("a-b/unique/p.md", "# Independent p\n"); + await write("a/nested/deep/p.md", "# Deep p\n"); + // Distinct even on Darwin's case-insensitive filesystem. These roots expose + // byte ordering that a locale comparator in the projection would override. + await section("Z", "system", "ordering", "Ordering winner"); + await section("b", "system", "ordering", "Ordering loser"); + await section("z-unicode", "system", "unicode", "Unicode winner"); + await section("é-unicode", "system", "unicode", "Unicode loser"); + + const site = createSite({ projectDir }); + const [sections, pages] = await Promise.all([site.listSections(), site.listPages()]); + expect( + sections + .filter((s) => s.sectionRef === shared) + .map((s) => s.path) + .sort(), + ).toEqual(["a", "a-b", "a/nested"].sort()); + await expect(site.pagePathFor(shared, "")).resolves.toBe("a"); + await expect(site.pagePathFor(ordering, "")).resolves.toBe("Z"); + await expect(site.pagePathFor(unicode, "")).resolves.toBe("z-unicode"); + expect(pages.find((p) => p.path === "a/nested/deep/p")?.anchors).toEqual([ + { sectionRef: deep, subpath: "p" }, + { sectionRef: shared, subpath: "deep/p" }, + { sectionRef: shared, subpath: "nested/deep/p" }, + { sectionRef: root, subpath: "a/nested/deep/p" }, + ]); + const projected = await projectSiteListing(sections, pages, (ref) => + site.pagePathFor(ref, ""), + ); + expect(projected.pages.map((p) => p.path).sort()).toEqual( + [ + "", + "a", + "a/q", + "a-b/unique", + "a-b/unique/p", + "a/nested/deep", + "a/nested/deep/p", + "Z", + "z-unicode", + ].sort(), + ); + expect( + pages + .filter((p) => !projected.pages.some((retained) => retained.path === p.path)) + .map((p) => p.path) + .sort(), + ).toEqual(["a-b", "a-b/q", "a-b/only", "a/nested", "b", "é-unicode"].sort()); + expect(projected.diagnostics).toEqual({ + collidingRefs: 3, + omittedSections: 4, + omittedPages: 6, + }); + expect(projected.sections.find((s) => s.sectionRef === independent)?.ancestors).toEqual([ + root, + ]); + expect(projected.sections.find((s) => s.sectionRef === deep)?.ancestors).toEqual([ + shared, + root, + ]); + expect(projected.pages.find((p) => p.path === "a-b/unique/p")?.anchors).toEqual([ + { sectionRef: independent, subpath: "p" }, + { sectionRef: root, subpath: "a-b/unique/p" }, + ]); + expect(projected.pages.find((p) => p.path === "a/nested/deep/p")?.anchors).toEqual([ + { sectionRef: deep, subpath: "p" }, + { sectionRef: shared, subpath: "nested/deep/p" }, + { sectionRef: root, subpath: "a/nested/deep/p" }, + ]); + for (const page of projected.pages) { + await expect(site.pagePathFor(page.sectionRef, page.subpath)).resolves.toBe(page.path); + const markdown = await site.getPageMarkdown(page.path); + expect(markdown).not.toBeNull(); + } + const qPath = await site.pagePathFor(shared, "q"); + expect(qPath).toBe("a/q"); + await expect(site.getPageMarkdown(qPath!)).resolves.toEqual({ markdown: winnerMarkdown }); + const reversed = await projectSiteListing( + [...sections].reverse(), + [...pages].reverse(), + (ref) => site.pagePathFor(ref, ""), + ); + expect(reversed.pages.map((p) => p.path).sort()).toEqual( + projected.pages.map((p) => p.path).sort(), + ); + } finally { + await rm(projectDir, { recursive: true, force: true }); + } + }); +}); diff --git a/plugins/rw-common/src/index.ts b/plugins/rw-common/src/index.ts index e300316..08fa3d7 100644 --- a/plugins/rw-common/src/index.ts +++ b/plugins/rw-common/src/index.ts @@ -16,3 +16,5 @@ export type { export { buildCommentDeepLinkSuffix, buildDocsPageLinkSuffix } from "./commentLink"; export { stringifySitePageRef, parseSitePageRef } from "./sitePageRef"; export type { SitePageRef } from "./sitePageRef"; +export { projectSiteListing } from "./siteListing"; +export type { ListingSection, ListingPage } from "./siteListing"; diff --git a/plugins/rw-common/src/siteListing.test.ts b/plugins/rw-common/src/siteListing.test.ts new file mode 100644 index 0000000..6b3e395 --- /dev/null +++ b/plugins/rw-common/src/siteListing.test.ts @@ -0,0 +1,179 @@ +import { projectSiteListing } from "./siteListing"; + +const root = "section:default/root"; +const shared = "system:default/shared"; +const independent = "component:default/independent"; +const deep = "component:default/deep"; +const section = (sectionRef: string, path: string, ancestors: string[] = []) => ({ + sectionRef, + path, + ancestors, +}); +const anchor = (sectionRef: string, subpath: string) => ({ sectionRef, subpath }); +const page = ( + sectionRef: string, + subpath: string, + path: string, + title = path, + anchors = [anchor(sectionRef, subpath), anchor(root, path)], +) => ({ + sectionRef, + subpath, + path, + title, + anchors, + hasContent: true, + lastModified: "2026-06-24T00:00:00Z", +}); + +function fixture() { + return { + sections: [ + section(root, ""), + section(shared, "a", [root]), + section(shared, "a-b", [root]), + section(independent, "a-b/unique", [shared, root]), + ], + pages: [ + page(root, "", "", "Home", [anchor(root, "")]), + page(shared, "", "a", "Winner"), + page(shared, "", "a-b", "Loser"), + page(shared, "q", "a/q", "Winner q"), + page(shared, "q", "a-b/q", "Loser q"), + page(shared, "only", "a-b/only", "Loser only"), + page(independent, "p", "a-b/unique/p", "Independent p", [ + anchor(independent, "p"), + anchor(shared, "unique/p"), + anchor(root, "a-b/unique/p"), + ]), + ], + }; +} + +describe("projectSiteListing", () => { + it.each(["original", "reversed", "shuffled"])( + "projects canonical content independent of listing order (%s)", + async (order) => { + const { sections, pages } = fixture(); + if (order === "reversed") { + sections.reverse(); + pages.reverse(); + } + if (order === "shuffled") { + sections.push(...sections.splice(0, 2)); + pages.push(...pages.splice(0, 3)); + } + const original = JSON.parse(JSON.stringify({ sections, pages })); + const resolve = jest.fn().mockResolvedValue("a"); + const projected = await projectSiteListing(sections, pages, resolve); + expect(projected.sections.find((s) => s.sectionRef === shared)?.path).toBe("a"); + expect(projected.pages.map((p) => p.path).sort()).toEqual( + ["", "a", "a-b/unique/p", "a/q"].sort(), + ); + expect(projected.sections.find((s) => s.sectionRef === independent)?.ancestors).toEqual([ + root, + ]); + expect(projected.pages.find((p) => p.path === "a-b/unique/p")?.anchors).toEqual([ + anchor(independent, "p"), + anchor(root, "a-b/unique/p"), + ]); + expect(projected.pages.find((p) => p.path === "a/q")).toMatchObject({ + title: "Winner q", + lastModified: "2026-06-24T00:00:00Z", + hasContent: true, + }); + expect(projected.diagnostics).toEqual({ + collidingRefs: 1, + omittedSections: 1, + omittedPages: 3, + }); + expect(resolve).toHaveBeenCalledTimes(1); + expect(resolve).toHaveBeenCalledWith(shared); + expect({ sections, pages }).toEqual(original); + }, + ); + + it("filters invalid repeated-ref anchors before keeping the valid outer anchor, using directory boundaries", async () => { + const { sections, pages } = fixture(); + sections.push( + section(shared, "a/nested", [shared, root]), + section(deep, "a/nested/deep", [shared, shared, root]), + section("component:default/sibling", "a2/unique", [root]), + ); + pages.push( + page(deep, "p", "a/nested/deep/p", "Deep p", [ + anchor(deep, "p"), + anchor(shared, "deep/p"), + anchor(shared, "nested/deep/p"), + anchor(root, "a/nested/deep/p"), + ]), + ); + const original = JSON.parse(JSON.stringify({ sections, pages })); + const projected = await projectSiteListing(sections, pages, async () => "a"); + expect(projected.sections.find((s) => s.sectionRef === deep)?.ancestors).toEqual([ + shared, + root, + ]); + expect(projected.sections.find((s) => s.path === "a2/unique")?.ancestors).toEqual([root]); + expect(projected.pages.find((p) => p.sectionRef === deep)?.anchors).toEqual([ + anchor(deep, "p"), + anchor(shared, "nested/deep/p"), + anchor(root, "a/nested/deep/p"), + ]); + expect({ sections, pages }).toEqual(original); + }); + + it.each([ + ["Z", "b"], + ["b", "Z"], + ["é", "中"], + ["中", "é"], + ["", "a"], + ])("delegates the winner exactly to the resolver: %s over %s", async (winner, loser) => { + for (const sections of [ + [section(shared, winner), section(shared, loser)], + [section(shared, loser), section(shared, winner)], + ]) { + const result = await projectSiteListing( + sections, + [page(shared, "", winner), page(shared, "", loser)], + async () => winner, + ); + expect(result.sections.map((s) => s.path)).toEqual([winner]); + expect(result.pages.map((p) => p.path)).toEqual([winner]); + } + }); + + it.each([null, "not-listed"])( + "rejects inconsistent canonical resolution %s with the ref", + async (result) => { + const { sections, pages } = fixture(); + await expect(projectSiteListing(sections, pages, async () => result)).rejects.toThrow( + `Cannot resolve canonical section root for ${shared}`, + ); + }, + ); + + it("propagates resolver rejection", async () => { + const { sections, pages } = fixture(); + const error = new Error("lookup failed"); + await expect( + projectSiteListing(sections, pages, async () => { + throw error; + }), + ).rejects.toBe(error); + }); + + it("never resolves unique refs and removes anchors and pages with unknown roots", async () => { + const resolve = jest.fn(); + const result = await projectSiteListing( + [section(root, "")], + [page(root, "p", "p", "P", [anchor(shared, "p"), anchor(root, "p")]), page(shared, "q", "q")], + resolve, + ); + expect(resolve).not.toHaveBeenCalled(); + expect(result.pages).toHaveLength(1); + expect(result.pages[0].anchors).toEqual([anchor(root, "p")]); + expect(result.diagnostics).toEqual({ collidingRefs: 0, omittedSections: 0, omittedPages: 1 }); + }); +}); diff --git a/plugins/rw-common/src/siteListing.ts b/plugins/rw-common/src/siteListing.ts new file mode 100644 index 0000000..e80cf14 --- /dev/null +++ b/plugins/rw-common/src/siteListing.ts @@ -0,0 +1,87 @@ +export interface ListingSection { + sectionRef: string; + path: string; + ancestors: string[]; +} + +export interface ListingPage { + sectionRef: string; + subpath: string; + path: string; + anchors: Array<{ sectionRef: string; subpath: string }>; +} + +function enclosingRefs(path: string, byPath: Map): string[] { + const refs: string[] = []; + let cursor = path; + while (cursor !== "") { + const slash = cursor.lastIndexOf("/"); + cursor = slash < 0 ? "" : cursor.slice(0, slash); + const ref = byPath.get(cursor); + if (ref !== undefined) refs.push(ref); + } + return refs; +} + +function joinedPath(root: string | undefined, subpath: string): string | undefined { + if (root === undefined) return undefined; + return root && subpath ? `${root}/${subpath}` : root || subpath; +} + +/** + * Projects physical listings onto canonical section identities. + * + * Uses `resolveSectionPath` to select roots for duplicate section refs. + * Independently named descendants remain, with only valid physical ancestry. + * + * @throws If a duplicate ref resolves to null or a path absent from its group. + * @throws Propagates errors from `resolveSectionPath`. + */ +export async function projectSiteListing( + sections: S[], + pages: P[], + resolveSectionPath: (sectionRef: string) => Promise, +): Promise<{ + sections: S[]; + pages: P[]; + diagnostics: { collidingRefs: number; omittedSections: number; omittedPages: number }; +}> { + const groups = new Map(); + for (const section of sections) { + const group = groups.get(section.sectionRef); + if (group) group.push(section); + else groups.set(section.sectionRef, [section]); + } + const selected = await Promise.all( + Array.from(groups, async ([ref, group]) => { + if (group.length === 1) return group[0]; + const path = await resolveSectionPath(ref); + const winner = path === null ? undefined : group.find((section) => section.path === path); + if (!winner) throw new Error(`Cannot resolve canonical section root for ${ref}`); + return winner; + }), + ); + const rootByRef = new Map(selected.map((section) => [section.sectionRef, section.path])); + const refByPath = new Map(selected.map((section) => [section.path, section.sectionRef])); + const projectedSections = selected.map((section) => ({ + ...section, + ancestors: enclosingRefs(section.path, refByPath), + })); + const projectedPages = pages + .filter((page) => joinedPath(rootByRef.get(page.sectionRef), page.subpath) === page.path) + .map((page) => ({ + ...page, + anchors: page.anchors.filter( + (anchor) => joinedPath(rootByRef.get(anchor.sectionRef), anchor.subpath) === page.path, + ), + })); + return { + sections: projectedSections, + pages: projectedPages, + diagnostics: { + collidingRefs: Array.from(groups.values()).filter((group) => group.length > 1).length, + omittedSections: sections.length - projectedSections.length, + omittedPages: pages.length - projectedPages.length, + }, + }; +} diff --git a/plugins/rw/package.json b/plugins/rw/package.json index 7772705..203af96 100644 --- a/plugins/rw/package.json +++ b/plugins/rw/package.json @@ -55,7 +55,7 @@ "@backstage/ui": "^0.17.0", "@material-ui/icons": "^4.11.3", "@rwdocs/backstage-plugin-rw-common": "workspace:^", - "@rwdocs/viewer": "^0.1.35" + "@rwdocs/viewer": "^0.1.36" }, "peerDependencies": { "@backstage/core-components": "^0.18.12", diff --git a/plugins/rw/src/api/RwClient.test.ts b/plugins/rw/src/api/RwClient.test.ts index 8119ef1..279695b 100644 --- a/plugins/rw/src/api/RwClient.test.ts +++ b/plugins/rw/src/api/RwClient.test.ts @@ -190,3 +190,57 @@ describe("RwClient comment methods", () => { expect(body.documentId).toBe("section:default/root#guide"); }); }); + +describe("RwClient.getSiteRootSectionRef", () => { + it.each([ + [{ kind: "section", namespace: "default", name: "root" }, "section:default/root"], + [{ kind: "section", namespace: "commerce", name: "root" }, "section:commerce/root"], + [{ kind: "domain", namespace: "Commerce", name: "Handbook" }, "domain:Commerce/Handbook"], + ])("reads the exact root identity %j", async (section, expected) => { + const { client, fetchMock } = makeClient(); + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ scope: { path: "/", section } }), + }); + await expect(client.getSiteRootSectionRef("default/component/arch")).resolves.toBe(expected); + expect(fetchMock).toHaveBeenCalledWith( + "http://backstage/api/rw/site/default/component/arch/navigation", + ); + }); + + it("rejects non-ok navigation responses", async () => { + const { client, fetchMock } = makeClient(); + fetchMock.mockResolvedValue({ ok: false, status: 404 }); + await expect(client.getSiteRootSectionRef("default/component/arch")).rejects.toThrow( + "Site navigation request failed: 404", + ); + }); + + it.each([ + null, + {}, + { scope: { path: "/child", section: { kind: "section", namespace: "default", name: "root" } } }, + { scope: { path: "/" } }, + ...["kind", "namespace", "name"].flatMap((field) => + [undefined, "", 42].map((value) => ({ + scope: { + path: "/", + section: { kind: "section", namespace: "default", name: "root", [field]: value }, + }, + })), + ), + ])("rejects invalid root navigation %j", async (body) => { + const { client, fetchMock } = makeClient(); + fetchMock.mockResolvedValue({ ok: true, json: async () => body }); + await expect(client.getSiteRootSectionRef("default/component/arch")).rejects.toThrow( + "Site navigation is missing a valid root section", + ); + }); + + it("propagates fetch rejection", async () => { + const { client, fetchMock } = makeClient(); + const error = new Error("network timeout"); + fetchMock.mockRejectedValue(error); + await expect(client.getSiteRootSectionRef("default/component/arch")).rejects.toBe(error); + }); +}); diff --git a/plugins/rw/src/api/RwClient.ts b/plugins/rw/src/api/RwClient.ts index d90028d..c6ea3e5 100644 --- a/plugins/rw/src/api/RwClient.ts +++ b/plugins/rw/src/api/RwClient.ts @@ -25,6 +25,7 @@ export type { export interface RwApi { getBaseUrl(): Promise; getSiteBaseUrl(entityRef: string): Promise; + getSiteRootSectionRef(entityPath: string): Promise; getFetch(): typeof fetch; getCommentsEnabled(): Promise; getCommentInbox(query?: InboxQuery): Promise; @@ -52,6 +53,25 @@ export class RwClient implements RwApi { return `${base}/site/${entityRef}`; } + async getSiteRootSectionRef(entityPath: string): Promise { + const base = await this.getSiteBaseUrl(entityPath); + const res = await this.fetchApi.fetch(`${base}/navigation`); + if (!res.ok) throw new Error(`Site navigation request failed: ${res.status}`); + const body = await res.json(); + const scope = body?.scope; + const section = scope?.section; + if ( + scope?.path !== "/" || + !section || + ["kind", "namespace", "name"].some( + (field) => typeof section[field] !== "string" || section[field].length === 0, + ) + ) { + throw new Error("Site navigation is missing a valid root section"); + } + return `${section.kind}:${section.namespace}/${section.name}`; + } + getFetch(): typeof fetch { return this.fetchApi.fetch; } diff --git a/plugins/rw/src/components/RwDocsViewer.test.tsx b/plugins/rw/src/components/RwDocsViewer.test.tsx index c9ee645..9c37bf7 100644 --- a/plugins/rw/src/components/RwDocsViewer.test.tsx +++ b/plugins/rw/src/components/RwDocsViewer.test.tsx @@ -24,7 +24,9 @@ const TEST_API_BASE_URL = "http://localhost:7007/api/rw/site/default/component/m const TEST_SOURCE_ENTITY_REF = "component:default/my-docs"; const mockCatalogApi = { - getEntityByRef: jest.fn().mockResolvedValue(undefined), + getEntitiesByRefs: jest.fn(async ({ entityRefs }: { entityRefs: string[] }) => ({ + items: entityRefs.map(() => undefined), + })), }; function createMockRwApi(overrides?: Partial): RwApi { @@ -35,6 +37,7 @@ function createMockRwApi(overrides?: Partial): RwApi { .mockImplementation((entityRef: string) => Promise.resolve(`http://localhost:7007/api/rw/site/${entityRef}`), ), + getSiteRootSectionRef: jest.fn().mockResolvedValue("section:commerce/handbook"), getFetch: jest.fn().mockReturnValue(jest.fn()), getCommentsEnabled: jest.fn().mockResolvedValue(false), getCommentInbox: jest.fn().mockResolvedValue({ @@ -63,6 +66,7 @@ function renderViewer(mockApi: RwApi, props?: { sectionRef?: string; sourceEntit ]} > { expect(result[TEST_SOURCE_ENTITY_REF]).toBe("/"); // Other ref not in catalog → omitted expect(result["component:default/other"]).toBeUndefined(); + expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["component:default/other"], + }); }); + it("maps a named root to the source docs tab while keeping scoped self at the current base", async () => { + const scope = "system:commerce/payments-api"; + await renderViewer(createMockRwApi(), { sectionRef: scope }); + const options = mockMountRw.mock.calls.at(-1)![1]; + expect(await options.resolveSectionRefs!(["section:commerce/handbook", scope])).toEqual({ + "section:commerce/handbook": "/catalog/default/component/my-docs/docs", + [scope]: "/", + }); + expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["section:commerce/handbook"], + }); + }); + + it.each(["source", "root"])( + "remounts with a current resolver when only %s changes", + async (change) => { + const mockApi = createMockRwApi(); + const element = (source: string, root: string) => ( + + + + ); + const { rerender } = await renderInTestApp( + element(TEST_SOURCE_ENTITY_REF, "section:commerce/handbook"), + ); + const source = change === "source" ? "component:default/other" : TEST_SOURCE_ENTITY_REF; + const root = change === "root" ? "domain:Commerce/Handbook" : "section:commerce/handbook"; + rerender(element(source, root)); + expect(mockDestroy).toHaveBeenCalledTimes(1); + expect(mockMountRw).toHaveBeenCalledTimes(2); + expect(await mockMountRw.mock.calls.at(-1)![1].resolveSectionRefs!([root])).toEqual({ + [root]: + change === "source" + ? "/catalog/default/component/other/docs" + : "/catalog/default/component/my-docs/docs", + }); + expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ entityRefs: [root] }); + }, + ); + it("calls destroy on unmount", async () => { const { unmount } = await renderViewer(createMockRwApi()); @@ -178,6 +235,7 @@ describe("RwDocsViewer", () => { ]} > (null); const rwApi = useApi(rwApiRef); const theme = useTheme(); const [error, setError] = useState(null); - const catalogResolver = useSectionRefResolver(sourceEntityRef); + const catalogResolver = useSectionRefResolver(sourceEntityRef, rootSectionRef); const location = useLocation(); const navigate = useNavigate(); @@ -92,7 +94,7 @@ export function RwDocsViewer({ instanceRef.current = null; }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [apiBaseUrl, sectionRef, comments]); + }, [apiBaseUrl, sectionRef, comments, sourceEntityRef, rootSectionRef]); useEffect(() => { instanceRef.current?.setColorScheme(theme.palette.type); diff --git a/plugins/rw/src/components/RwEntityDocsViewer.test.tsx b/plugins/rw/src/components/RwEntityDocsViewer.test.tsx index a13efb0..07be6c4 100644 --- a/plugins/rw/src/components/RwEntityDocsViewer.test.tsx +++ b/plugins/rw/src/components/RwEntityDocsViewer.test.tsx @@ -1,4 +1,4 @@ -import { screen, waitFor } from "@testing-library/react"; +import { act, screen, waitFor } from "@testing-library/react"; import { renderInTestApp, TestApiProvider } from "@backstage/test-utils"; import { catalogApiRef, EntityProvider } from "@backstage/plugin-catalog-react"; import { Entity } from "@backstage/catalog-model"; @@ -10,7 +10,9 @@ import { mountRw } from "@rwdocs/viewer"; const mockMountRw = mountRw as jest.MockedFunction; const mockCatalogApi = { - getEntityByRef: jest.fn().mockResolvedValue(undefined), + getEntitiesByRefs: jest.fn(async ({ entityRefs }: { entityRefs: string[] }) => ({ + items: entityRefs.map(() => undefined), + })), }; jest.mock("@rwdocs/viewer/embed.css", () => ({})); @@ -29,6 +31,7 @@ function createMockRwApi(overrides?: Partial): RwApi { .mockImplementation((entityRef: string) => Promise.resolve(`http://localhost:7007/api/rw/site/${entityRef}`), ), + getSiteRootSectionRef: jest.fn().mockResolvedValue("section:commerce/handbook"), getFetch: jest.fn().mockReturnValue(jest.fn()), getCommentsEnabled: jest.fn().mockResolvedValue(false), getCommentInbox: jest.fn().mockResolvedValue({ @@ -75,6 +78,16 @@ function makeApisElement(mockApi: RwApi, entity: Entity) { ); } +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + describe("RwEntityDocsViewer", () => { beforeEach(() => { jest.clearAllMocks(); @@ -129,6 +142,305 @@ describe("RwEntityDocsViewer", () => { }); }); + it.each([ + [".", "section:commerce/handbook"], + [".#system:commerce/payments-api", "system:commerce/payments-api"], + ])("mounts %s with scope %s", async (annotation, sectionRef) => { + const mockApi = createMockRwApi(); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": annotation }))); + await waitFor(() => expect(mockMountRw).toHaveBeenCalled()); + expect(mockMountRw.mock.calls.at(-1)![1].sectionRef).toBe(sectionRef); + expect(mockApi.getSiteRootSectionRef).toHaveBeenCalledWith("default/component/my-service"); + }); + + it("waits for the required root even when URL and comments are available", async () => { + let resolveRoot!: (root: string) => void; + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest.fn().mockReturnValue( + new Promise((resolve) => { + resolveRoot = resolve; + }), + ), + }); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "." }))); + expect(screen.getByTestId("progress")).toBeInTheDocument(); + expect(mockMountRw).not.toHaveBeenCalled(); + await act(async () => { + resolveRoot("section:commerce/handbook"); + }); + expect(mockMountRw.mock.calls.at(-1)![1].sectionRef).toBe("section:commerce/handbook"); + }); + + it.each(["root", "comments"])( + "starts comments while root is pending and gates mounting when %s settles first", + async (first) => { + const root = deferred(); + const comments = deferred(); + const client = { list: jest.fn(), create: jest.fn(), update: jest.fn(), delete: jest.fn() }; + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest.fn().mockReturnValue(root.promise), + getCommentsEnabled: jest.fn().mockReturnValue(comments.promise), + createCommentClient: jest.fn().mockReturnValue(client), + }); + await renderInTestApp( + makeApisElement( + mockApi, + makeEntity({ "rwdocs.org/ref": ".#system:commerce/payments-api" }), + ), + ); + expect(mockApi.getCommentsEnabled).toHaveBeenCalledTimes(1); + expect(mockMountRw).not.toHaveBeenCalled(); + await act(async () => { + if (first === "root") root.resolve("domain:commerce/handbook"); + else comments.resolve(true); + }); + expect(screen.getByTestId("progress")).toBeInTheDocument(); + expect(mockMountRw).not.toHaveBeenCalled(); + await act(async () => { + if (first === "root") comments.resolve(true); + else root.resolve("domain:commerce/handbook"); + }); + expect(mockMountRw).toHaveBeenCalledTimes(1); + const options = mockMountRw.mock.calls[0][1]; + expect(options.sectionRef).toBe("system:commerce/payments-api"); + expect(options.comments).toBe(client); + expect(mockApi.createCommentClient).toHaveBeenCalledWith("component:default/my-service"); + expect(await options.resolveSectionRefs!(["domain:commerce/handbook"])).toEqual({ + "domain:commerce/handbook": "/catalog/default/component/my-service/docs", + }); + expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["domain:commerce/handbook"], + }); + }, + ); + + it("degrades a comments rejection while root is pending, then mounts when root succeeds", async () => { + const root = deferred(); + const comments = deferred(); + const error = new Error("optional probe failed"); + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + try { + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest.fn().mockReturnValue(root.promise), + getCommentsEnabled: jest.fn().mockReturnValue(comments.promise), + }); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "." }))); + expect(mockApi.getCommentsEnabled).toHaveBeenCalledTimes(1); + await act(async () => { + comments.reject(error); + }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + "rw: comments-enabled probe failed; comments disabled for this view", + error, + ); + expect(mockMountRw).not.toHaveBeenCalled(); + await act(async () => { + root.resolve("domain:commerce/handbook"); + }); + expect(mockMountRw).toHaveBeenCalledTimes(1); + expect(mockMountRw.mock.calls[0][1].sectionRef).toBe("domain:commerce/handbook"); + expect(mockMountRw.mock.calls[0][1].comments).toBeUndefined(); + } finally { + warn.mockRestore(); + } + }); + + it.each(["success", "rejection"])( + "retains required root error after late comments %s", + async (outcome) => { + const root = deferred(); + const comments = deferred(); + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + try { + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest.fn().mockReturnValue(root.promise), + getCommentsEnabled: jest.fn().mockReturnValue(comments.promise), + }); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "." }))); + expect(mockApi.getCommentsEnabled).toHaveBeenCalledTimes(1); + await act(async () => { + root.reject(new Error("required root failed")); + }); + expect(screen.getAllByText(/required root failed/).length).toBeGreaterThan(0); + expect(mockMountRw).not.toHaveBeenCalled(); + await act(async () => { + if (outcome === "success") comments.resolve(true); + else comments.reject(new Error("late optional failure")); + }); + expect(screen.getAllByText(/required root failed/).length).toBeGreaterThan(0); + expect(mockMountRw).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }, + ); + + it.each(["success", "rejection"])( + "suppresses obsolete comments %s after a site switch", + async (outcome) => { + const rootA = deferred(); + const commentsA = deferred(); + const clientB = { list: jest.fn(), create: jest.fn(), update: jest.fn(), delete: jest.fn() }; + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + try { + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest + .fn() + .mockReturnValueOnce(rootA.promise) + .mockResolvedValue("domain:commerce/home-b"), + getCommentsEnabled: jest + .fn() + .mockReturnValueOnce(commentsA.promise) + .mockResolvedValue(true), + createCommentClient: jest.fn().mockReturnValue(clientB), + }); + const { rerender } = await renderInTestApp( + makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "component:default/site-a" })), + ); + expect(mockApi.getCommentsEnabled).toHaveBeenCalledTimes(1); + rerender( + makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "component:default/site-b" })), + ); + await waitFor(() => expect(mockMountRw).toHaveBeenCalledTimes(1)); + await act(async () => { + if (outcome === "success") commentsA.resolve(true); + else commentsA.reject(new Error("obsolete optional failure")); + rootA.resolve("domain:commerce/home-a"); + }); + expect(warn).not.toHaveBeenCalled(); + expect(mockApi.createCommentClient).toHaveBeenCalledTimes(1); + expect(mockApi.createCommentClient).toHaveBeenCalledWith("component:default/site-b"); + expect(mockMountRw).toHaveBeenCalledTimes(1); + expect(mockMountRw.mock.calls[0][1]).toMatchObject({ + apiBaseUrl: "http://localhost:7007/api/rw/site/default/component/site-b", + sectionRef: "domain:commerce/home-b", + comments: clientB, + }); + } finally { + warn.mockRestore(); + } + }, + ); + + it("degrades a comment client factory failure", async () => { + const error = new Error("client factory failed"); + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + try { + const mockApi = createMockRwApi({ + getCommentsEnabled: jest.fn().mockResolvedValue(true), + createCommentClient: jest.fn().mockImplementation(() => { + throw error; + }), + }); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "." }))); + expect(mockMountRw).toHaveBeenCalledTimes(1); + expect(mockMountRw.mock.calls[0][1].comments).toBeUndefined(); + expect(warn).toHaveBeenCalledWith( + "rw: comments-enabled probe failed; comments disabled for this view", + error, + ); + } finally { + warn.mockRestore(); + } + }); + + it("shows required root errors without mounting", async () => { + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest.fn().mockRejectedValue(new Error("root navigation unavailable")), + }); + await renderInTestApp(makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "." }))); + expect(screen.getAllByText(/root navigation unavailable/).length).toBeGreaterThan(0); + expect(mockMountRw).not.toHaveBeenCalled(); + }); + + it("ignores site A's late root after site B has mounted", async () => { + let resolveA!: (root: string) => void; + let resolveB!: (root: string) => void; + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest + .fn() + .mockReturnValueOnce( + new Promise((resolve) => { + resolveA = resolve; + }), + ) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveB = resolve; + }), + ), + }); + const { rerender } = await renderInTestApp( + makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "component:default/site-a" })), + ); + rerender( + makeApisElement( + mockApi, + makeEntity({ "rwdocs.org/ref": "component:default/site-b#system:commerce/payments-api" }), + ), + ); + await waitFor(() => + expect(mockApi.getSiteRootSectionRef).toHaveBeenCalledWith("default/component/site-b"), + ); + await act(async () => { + resolveB("section:commerce/home-b"); + }); + const callsAfterB = mockMountRw.mock.calls.length; + await act(async () => { + resolveA("section:commerce/home-a"); + }); + expect(mockMountRw).toHaveBeenCalledTimes(callsAfterB); + const options = mockMountRw.mock.calls.at(-1)![1]; + expect(options.apiBaseUrl).toBe("http://localhost:7007/api/rw/site/default/component/site-b"); + expect(options.sectionRef).toBe("system:commerce/payments-api"); + expect(await options.resolveSectionRefs!(["section:commerce/home-b"])).toEqual({ + "section:commerce/home-b": "/catalog/default/component/site-b/docs", + }); + expect(mockCatalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["section:commerce/home-b"], + }); + }); + + it("never mounts site A's ready URL with site B's scope during a switch", async () => { + let resolveB!: (root: string) => void; + const mockApi = createMockRwApi({ + getSiteRootSectionRef: jest + .fn() + .mockResolvedValueOnce("section:commerce/home-a") + .mockReturnValueOnce( + new Promise((resolve) => { + resolveB = resolve; + }), + ), + }); + const { rerender } = await renderInTestApp( + makeApisElement(mockApi, makeEntity({ "rwdocs.org/ref": "component:default/site-a" })), + ); + await waitFor(() => expect(mockMountRw).toHaveBeenCalled()); + const callsAfterA = mockMountRw.mock.calls.length; + rerender( + makeApisElement( + mockApi, + makeEntity({ "rwdocs.org/ref": "component:default/site-b#system:commerce/payments-api" }), + ), + ); + expect(screen.getByTestId("progress")).toBeInTheDocument(); + expect(mockMountRw).toHaveBeenCalledTimes(callsAfterA); + await act(async () => { + resolveB("section:commerce/home-b"); + }); + expect( + mockMountRw.mock.calls.map(([, options]) => [options.apiBaseUrl, options.sectionRef]), + ).toEqual([ + ["http://localhost:7007/api/rw/site/default/component/site-a", "section:commerce/home-a"], + [ + "http://localhost:7007/api/rw/site/default/component/site-b", + "system:commerce/payments-api", + ], + ]); + }); + it("resolves base URL using source entity ref from annotation", async () => { const mockApi = createMockRwApi(); const entity = makeEntity({ "rwdocs.org/ref": "component:default/other-docs" }); diff --git a/plugins/rw/src/components/RwEntityDocsViewer.tsx b/plugins/rw/src/components/RwEntityDocsViewer.tsx index 68809bd..3bbd297 100644 --- a/plugins/rw/src/components/RwEntityDocsViewer.tsx +++ b/plugins/rw/src/components/RwEntityDocsViewer.tsx @@ -9,16 +9,20 @@ import { ANNOTATION_KEY } from "./constants"; import { RwDocsViewer } from "./RwDocsViewer"; import type { CommentApiClient } from "@rwdocs/viewer"; +type SiteSetup = + | { + entityPath: string; + status: "ready"; + apiBaseUrl: string; + rootSectionRef: string; + comments?: CommentApiClient; + } + | { entityPath: string; status: "error"; error: Error }; + export function RwEntityDocsViewer() { const { entity } = useEntity(); const rwApi = useApi(rwApiRef); - const [apiBaseUrl, setApiBaseUrl] = useState(null); - const [fetchError, setFetchError] = useState(null); - const [commentClient, setCommentClient] = useState(undefined); - // Two gates before the viewer mounts: apiBaseUrl resolves first, then we wait - // for the comments-enabled check so the viewer never mounts and immediately - // remounts with a different comments prop. - const [commentsReady, setCommentsReady] = useState(false); + const [setup, setSetup] = useState(); const annotationValue = entity.metadata.annotations?.[ANNOTATION_KEY]; const selfEntityRef = useMemo(() => toEntityPath(getCompoundEntityRef(entity)), [entity]); @@ -27,35 +31,43 @@ export function RwEntityDocsViewer() { useEffect(() => { if (!parsed) return undefined; - // Reset gate immediately so the viewer unmounts while we fetch the new entity's data. - setApiBaseUrl(null); - setFetchError(null); - setCommentsReady(false); - setCommentClient(undefined); + const { entityPath, entityRef } = parsed; + setSetup(undefined); let cancelled = false; - (async () => { + const optionalComments = (async (): Promise => { try { - const url = await rwApi.getSiteBaseUrl(parsed.entityPath); - if (cancelled) return; - setApiBaseUrl(url); + const enabled = await rwApi.getCommentsEnabled(); + if (cancelled) return undefined; + return enabled ? rwApi.createCommentClient(entityRef) : undefined; } catch (err) { - if (!cancelled) setFetchError(err instanceof Error ? err : new Error(String(err))); - return; + if (!cancelled) { + // eslint-disable-next-line no-console + console.warn("rw: comments-enabled probe failed; comments disabled for this view", err); + } + return undefined; } + })(); + (async () => { try { - const enabled = await rwApi.getCommentsEnabled(); - if (cancelled) return; - setCommentClient(enabled ? rwApi.createCommentClient(parsed.entityRef) : undefined); + const [apiBaseUrl, rootSectionRef, comments] = await Promise.all([ + rwApi.getSiteBaseUrl(entityPath), + rwApi.getSiteRootSectionRef(entityPath), + optionalComments, + ]); + + if (!cancelled) + setSetup({ entityPath, status: "ready", apiBaseUrl, rootSectionRef, comments }); } catch (err) { - if (cancelled) return; - // eslint-disable-next-line no-console - console.warn("rw: comments-enabled probe failed; comments disabled for this view", err); - setCommentClient(undefined); + if (!cancelled) { + setSetup({ + entityPath, + status: "error", + error: err instanceof Error ? err : new Error(String(err)), + }); + } } - - if (!cancelled) setCommentsReady(true); })(); return () => { cancelled = true; @@ -67,21 +79,22 @@ export function RwEntityDocsViewer() { return ; } - if (fetchError) { - return ; + if (!setup || setup.entityPath !== parsed.entityPath) { + return ; } - if (!apiBaseUrl || !commentsReady) { - return ; + if (setup.status === "error") { + return ; } - const sectionRef = parsed.sectionRef ?? selfEntityRef; + const sectionRef = parsed.sectionRef ?? setup.rootSectionRef; return ( ); } diff --git a/plugins/rw/src/components/constants.ts b/plugins/rw/src/components/constants.ts index 2fcaea8..5907e4e 100644 --- a/plugins/rw/src/components/constants.ts +++ b/plugins/rw/src/components/constants.ts @@ -1,27 +1,5 @@ -import { parseEntityRef } from "@backstage/catalog-model"; - export const ANNOTATION_KEY = "rwdocs.org/ref"; -/** rw's implicit site-root section is always `section:/root` — kind - * "section", name "root", carrying the site's (possibly custom) namespace. */ -const ROOT_SECTION_KIND = "section"; -const ROOT_SECTION_NAME = "root"; - -/** Whether a section ref is the site-root section — the ancestry backstop the - * viewer resolves every cross-entity link against, which the host must map to - * the root/site entity. Matched by kind+name (not the literal - * `section:default/root`) so a custom-namespace root like `section:acme/root` - * resolves too, while a content section merely named "root" (e.g. a - * `domain:default/root` folder) is left to catalog resolution. */ -export function isRootSectionRef(ref: string): boolean { - try { - const { kind, name } = parseEntityRef(ref); - return kind === ROOT_SECTION_KIND && name === ROOT_SECTION_NAME; - } catch { - return false; - } -} - /** The entity content-tab path segment that mounts the RW docs viewer (the * `rwEntityContent` EntityContentBlueprint's `path: "docs"`). */ export const DOCS_PATH_SUFFIX = "/docs"; diff --git a/plugins/rw/src/components/useSectionRefResolver.test.tsx b/plugins/rw/src/components/useSectionRefResolver.test.tsx index 969a16d..e7d44db 100644 --- a/plugins/rw/src/components/useSectionRefResolver.test.tsx +++ b/plugins/rw/src/components/useSectionRefResolver.test.tsx @@ -4,7 +4,7 @@ import { TestApiProvider } from "@backstage/test-utils"; import { catalogApiRef } from "@backstage/plugin-catalog-react"; import { useSectionRefResolver } from "./useSectionRefResolver"; import { ANNOTATION_KEY } from "./constants"; -import type { Entity } from "@backstage/catalog-model"; +import { parseEntityRef, type Entity } from "@backstage/catalog-model"; const mockEntityRoute = jest.fn( ({ kind, namespace, name }: { kind: string; namespace: string; name: string }) => @@ -23,11 +23,12 @@ jest.mock("@backstage/core-plugin-api", () => ({ const SOURCE_ENTITY_REF = "component:default/arch"; -function makeEntity(annotations?: Record): Entity { +function makeEntity(annotations?: Record, ref = "domain:default/billing"): Entity { + const { kind, namespace, name } = parseEntityRef(ref); return { apiVersion: "backstage.io/v1alpha1", - kind: "Domain", - metadata: { name: "billing", namespace: "default", annotations }, + kind, + metadata: { name, namespace, annotations }, }; } @@ -41,11 +42,18 @@ function createMockCatalogApi(entities: Record) { }; } -function renderWithCatalog(catalogApi: { getEntitiesByRefs: jest.Mock }) { +function renderWithCatalog( + catalogApi: { getEntitiesByRefs: jest.Mock }, + rootSectionRef = "section:default/root", + sourceEntityRef = SOURCE_ENTITY_REF, +) { const wrapper = ({ children }: { children: ReactNode }) => ( {children} ); - return renderHook(() => useSectionRefResolver(SOURCE_ENTITY_REF), { wrapper }); + return renderHook(({ source, root }) => useSectionRefResolver(source, root), { + wrapper, + initialProps: { source: sourceEntityRef, root: rootSectionRef }, + }); } describe("useSectionRefResolver", () => { @@ -71,6 +79,41 @@ describe("useSectionRefResolver", () => { }); }); + it("preserves ordinary URLs when optional root validation rejects a catalog self-path", async () => { + const refs = ["domain:default/billing+legacy", "domain:default/ok"]; + const catalogApi = createMockCatalogApi({ + [refs[0]]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, refs[0]), + [refs[1]]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, refs[1]), + }); + const { result } = renderWithCatalog(catalogApi, "section:default/root"); + + expect(await result.current(refs)).toEqual({ + "domain:default/billing+legacy": "/catalog/default/domain/billing+legacy/docs", + "domain:default/ok": "/catalog/default/domain/ok/docs", + }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ entityRefs: refs }); + }); + + it("caches a rejected self-path without making it an eligible root across source changes", async () => { + const ref = "domain:default/billing+legacy"; + const catalogApi = createMockCatalogApi({ + [ref]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, ref), + }); + const { result, rerender } = renderWithCatalog(catalogApi); + expect(await result.current([ref])).toEqual({ + [ref]: "/catalog/default/domain/billing+legacy/docs", + }); + rerender({ source: SOURCE_ENTITY_REF, root: ref }); + expect(await result.current([ref])).toEqual({ + [ref]: "/catalog/default/component/arch/docs", + }); + rerender({ source: "component:default/other", root: ref }); + expect(await result.current([ref])).toEqual({ + [ref]: "/catalog/default/component/other/docs", + }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledTimes(1); + }); + it("resolves root section ref to the source entity", async () => { const catalogApi = createMockCatalogApi({}); const { result } = renderWithCatalog(catalogApi); @@ -80,35 +123,32 @@ describe("useSectionRefResolver", () => { resolved = await result.current(["section:default/root"]); }); - expect(catalogApi.getEntitiesByRefs).not.toHaveBeenCalled(); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["section:default/root"], + }); expect(resolved).toEqual({ "section:default/root": "/catalog/default/component/arch/docs", }); }); it("resolves a custom-namespace root section ref to the source entity", async () => { - // rw names every site-root section "root" but carries the site's namespace, - // so a custom-namespace site emits section:/root, not section:default/root. - // The host must still map it to the root entity (the ancestry backstop). const catalogApi = createMockCatalogApi({}); - const { result } = renderWithCatalog(catalogApi); + const { result } = renderWithCatalog(catalogApi, "section:acme/root"); let resolved: Record = {}; await act(async () => { resolved = await result.current(["section:acme/root"]); }); - expect(catalogApi.getEntitiesByRefs).not.toHaveBeenCalled(); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["section:acme/root"], + }); expect(resolved).toEqual({ "section:acme/root": "/catalog/default/component/arch/docs", }); }); it("does not treat a non-section entity named 'root' as the site root", async () => { - // Only the implicit site-root section (kind "section") is the backstop. A - // content section whose last path segment is "root" (e.g. a docs/root/ folder - // with kind: domain) must be resolved through the catalog, not short-circuited - // to the source entity. const entity = makeEntity({ [ANNOTATION_KEY]: "." }); const catalogApi = createMockCatalogApi({ "domain:default/root": entity }); const { result } = renderWithCatalog(catalogApi); @@ -126,6 +166,135 @@ describe("useSectionRefResolver", () => { }); }); + it.each(["section:commerce/handbook", "domain:commerce/Handbook"])( + "falls back to the source for the exact authoritative root %s missing from catalog", + async (rootSectionRef) => { + const catalogApi = createMockCatalogApi({}); + const { result } = renderWithCatalog(catalogApi, rootSectionRef); + let resolved: Record = {}; + await act(async () => { + resolved = await result.current([rootSectionRef]); + }); + expect(resolved).toEqual({ [rootSectionRef]: "/catalog/default/component/arch/docs" }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ entityRefs: [rootSectionRef] }); + }, + ); + + it("resolves a non-root section:default/root through the catalog", async () => { + const catalogApi = createMockCatalogApi({ + "section:default/root": makeEntity({ [ANNOTATION_KEY]: "." }), + }); + const { result } = renderWithCatalog(catalogApi, "section:commerce/handbook"); + let resolved: Record = {}; + await act(async () => { + resolved = await result.current(["section:default/root"]); + }); + expect(resolved).toEqual({ "section:default/root": "/catalog/default/section/root/docs" }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: ["section:default/root"], + }); + }); + + it("preserves the catalog root view when the source is scoped (old-version regression)", async () => { + const root = "domain:default/root"; + const catalogApi = createMockCatalogApi({ + [SOURCE_ENTITY_REF]: makeEntity( + { [ANNOTATION_KEY]: ".#system:default/payments" }, + SOURCE_ENTITY_REF, + ), + [root]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, root), + }); + const { result } = renderWithCatalog(catalogApi, root); + expect(await result.current([root, "system:default/payments"])).toEqual({ + [root]: "/catalog/default/domain/root/docs", + }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledWith({ + entityRefs: [root, "system:default/payments"], + }); + }); + + it.each([ + ["another site", "component:default/other"], + ["same site, non-root fragment", "component:default/arch#system:default/payments"], + ["missing annotation", undefined], + ["invalid annotation", "///invalid///"], + ["catalog self is not the source", "."], + ["root fragment with different case", "component:default/arch#domain:default/Root"], + ])("falls back for %s", async (_description, annotation) => { + const root = "domain:default/root"; + const catalogApi = createMockCatalogApi({ + [root]: makeEntity(annotation ? { [ANNOTATION_KEY]: annotation } : {}, root), + }); + const { result } = renderWithCatalog(catalogApi, root); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/component/arch/docs", + }); + }); + + it("accepts a same-site explicit-root fragment", async () => { + const root = "domain:commerce/Handbook"; + const catalogApi = createMockCatalogApi({ + [root]: makeEntity( + { [ANNOTATION_KEY]: "component:default/arch#domain:commerce/Handbook" }, + root, + ), + }); + const { result } = renderWithCatalog(catalogApi, root); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/commerce/domain/Handbook/docs", + }); + }); + + it("re-evaluates cached catalog data across source and root changes", async () => { + const root = "domain:default/root"; + const catalogApi = createMockCatalogApi({ + [root]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, root), + }); + const { result, rerender } = renderWithCatalog(catalogApi, root); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/domain/root/docs", + }); + rerender({ source: "component:default/other", root }); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/component/other/docs", + }); + rerender({ source: "component:default/other", root: "section:commerce/new-home" }); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/domain/root/docs", + }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledTimes(1); + }); + + it("caches a missing root without retaining a previous source fallback", async () => { + const root = "section:commerce/handbook"; + const catalogApi = createMockCatalogApi({}); + const { result, rerender } = renderWithCatalog(catalogApi, root); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/component/arch/docs", + }); + rerender({ source: "component:default/other", root }); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/component/other/docs", + }); + rerender({ source: "component:default/other", root: "section:commerce/new-home" }); + expect(await result.current([root])).toEqual({}); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledTimes(1); + }); + + it("falls back after a failed root lookup and retries it", async () => { + const root = "domain:default/root"; + const catalogApi = createMockCatalogApi({ + [root]: makeEntity({ [ANNOTATION_KEY]: "component:default/arch" }, root), + }); + catalogApi.getEntitiesByRefs.mockRejectedValueOnce(new Error("network error")); + const { result } = renderWithCatalog(catalogApi, root); + expect(await result.current([root])).toEqual({ + [root]: "/catalog/default/component/arch/docs", + }); + expect(await result.current([root])).toEqual({ [root]: "/catalog/default/domain/root/docs" }); + expect(catalogApi.getEntitiesByRefs).toHaveBeenCalledTimes(2); + }); + it("returns empty map for entities without rwdocs annotation", async () => { const entity = makeEntity({}); const catalogApi = createMockCatalogApi({ "domain:default/billing": entity }); diff --git a/plugins/rw/src/components/useSectionRefResolver.ts b/plugins/rw/src/components/useSectionRefResolver.ts index c06138e..7f9d49f 100644 --- a/plugins/rw/src/components/useSectionRefResolver.ts +++ b/plugins/rw/src/components/useSectionRefResolver.ts @@ -2,40 +2,42 @@ import { useCallback, useRef } from "react"; import { useApi, useRouteRef } from "@backstage/core-plugin-api"; import { catalogApiRef, entityRouteRef } from "@backstage/plugin-catalog-react"; import { parseEntityRef } from "@backstage/catalog-model"; -import { ANNOTATION_KEY, isRootSectionRef, entityDocsPath } from "./constants"; +import { + parseAnnotation, + toEntityPath, + type ParsedAnnotation, +} from "@rwdocs/backstage-plugin-rw-common"; +import { ANNOTATION_KEY, entityDocsPath } from "./constants"; export function useSectionRefResolver( sourceEntityRef: string, + rootSectionRef: string, ): (refs: string[]) => Promise> { const catalogApi = useApi(catalogApiRef); const entityRoute = useRouteRef(entityRouteRef); - const cache = useRef(new Map()); + const cache = useRef(new Map()); return useCallback( async (refs: string[]): Promise> => { - const unknown = refs.filter((r) => !cache.current.has(r)); + const unknown = refs.filter((ref) => !cache.current.has(ref)); - const catalogRefs: string[] = []; - for (const ref of unknown) { - if (isRootSectionRef(ref)) { - const { kind, namespace, name } = parseEntityRef(sourceEntityRef); - const routeUrl = entityDocsPath(entityRoute, { kind, namespace, name }); - cache.current.set(ref, routeUrl); - } else { - catalogRefs.push(ref); - } - } - - if (catalogRefs.length > 0) { + if (unknown.length > 0) { try { - const { items } = await catalogApi.getEntitiesByRefs({ entityRefs: catalogRefs }); - for (let i = 0; i < catalogRefs.length; i++) { - const ref = catalogRefs[i]; + const { items } = await catalogApi.getEntitiesByRefs({ entityRefs: unknown }); + for (let i = 0; i < unknown.length; i++) { + const ref = unknown[i]; const entity = items[i]; - if (entity?.metadata.annotations?.[ANNOTATION_KEY]) { + const annotationValue = entity?.metadata.annotations?.[ANNOTATION_KEY]; + if (annotationValue) { const { kind, namespace, name } = parseEntityRef(ref); const routeUrl = entityDocsPath(entityRoute, { kind, namespace, name }); - cache.current.set(ref, routeUrl); + let annotation: ParsedAnnotation | undefined; + try { + annotation = parseAnnotation(annotationValue, toEntityPath(ref)); + } catch { + // Optional root validation must not discard ordinary catalog URLs. + } + cache.current.set(ref, { url: routeUrl, annotation }); } else { cache.current.set(ref, null); } @@ -47,13 +49,26 @@ export function useSectionRefResolver( const result: Record = {}; for (const ref of refs) { - const url = cache.current.get(ref); - if (url !== null && url !== undefined) { - result[ref] = url; + const cached = cache.current.get(ref); + if (ref === rootSectionRef) { + // Eligibility depends on this callback's site/root, not the cached lookup's context. + const annotation = cached?.annotation; + if ( + cached && + annotation?.entityPath === toEntityPath(sourceEntityRef) && + (!annotation.sectionRef || annotation.sectionRef === rootSectionRef) + ) { + result[ref] = cached.url; + } else { + const { kind, namespace, name } = parseEntityRef(sourceEntityRef); + result[ref] = entityDocsPath(entityRoute, { kind, namespace, name }); + } + } else if (cached) { + result[ref] = cached.url; } } return result; }, - [catalogApi, entityRoute, sourceEntityRef], + [catalogApi, entityRoute, sourceEntityRef, rootSectionRef], ); } diff --git a/plugins/search-backend-module-rw/package.json b/plugins/search-backend-module-rw/package.json index 1bc9eef..bdc47d7 100644 --- a/plugins/search-backend-module-rw/package.json +++ b/plugins/search-backend-module-rw/package.json @@ -54,7 +54,7 @@ "@backstage/plugin-permission-common": "^0.9.9", "@backstage/plugin-search-common": "^1.2.24", "@rwdocs/backstage-plugin-rw-common": "workspace:^", - "@rwdocs/core": "^0.1.35" + "@rwdocs/core": "^0.1.36" }, "peerDependencies": { "@backstage/backend-plugin-api": "^1.9.3", diff --git a/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.test.ts b/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.test.ts index d219eba..4a0e34b 100644 --- a/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.test.ts +++ b/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.test.ts @@ -20,6 +20,7 @@ interface MockSection { interface MockPage { sectionRef: string; subpath: string; + path?: string; } /** A site that hands out pages the way `@rwdocs/core` does: each page carries its @@ -34,6 +35,7 @@ function createMockSite(options: { sections?: MockSection[]; pages?: MockPage[]; documents?: Record; + canonicalRoots?: Record; }) { const sections = (options.sections ?? []).map((section) => ({ ancestors: [], ...section })); const byRef = new Map(sections.map((section) => [section.sectionRef, section])); @@ -48,21 +50,30 @@ function createMockSite(options: { const pages = (options.pages ?? []).map((page) => { const section = byRef.get(page.sectionRef)!; - const path = join(section.path, page.subpath); + const path = page.path ?? join(section.path, page.subpath); return { ...page, path, title: "", lastModified: "2026-07-12T00:00:00+00:00", hasContent: docs[path] !== null, - anchors: [section.sectionRef, ...section.ancestors].map((ref) => ({ - sectionRef: ref, - subpath: relativeTo(path, byRef.get(ref)?.path ?? ""), - })), + anchors: sections + .filter((s) => s.path === "" || path === s.path || path.startsWith(`${s.path}/`)) + .sort((a, b) => b.path.length - a.path.length) + .map((s) => ({ sectionRef: s.sectionRef, subpath: relativeTo(path, s.path) })), }; }); return { + listSections: jest.fn().mockResolvedValue(sections), + pagePathFor: jest.fn(async (ref: string, subpath: string) => { + const matches = sections.filter((s) => s.sectionRef === ref); + if (!matches.length) return null; + const root = options.canonicalRoots?.[ref]; + if (matches.length > 1 && root === undefined) + throw new Error(`Fixture must prescribe canonical root for ${ref}`); + return join(root ?? matches[0].path, subpath); + }), listPages: jest.fn().mockResolvedValue(pages), renderSearchDocument: jest.fn().mockImplementation(async (path: string) => docs[path] ?? null), } as any; @@ -194,7 +205,7 @@ describe("RwDocsCollatorFactory", () => { }, { sectionRef: "system:default/payment-gateway", - path: "domains/billing/systems/payment-gateway", + path: "domains/billing/systems/payments-guide", ancestors: ["domain:default/billing", "section:default/root"], }, ], @@ -207,8 +218,8 @@ describe("RwDocsCollatorFactory", () => { documents: { guide: { title: "Guide", text: "Guide" }, "domains/billing/overview": { title: "Overview", text: "Overview" }, - "domains/billing/systems/payment-gateway": { title: "Payment Gateway", text: "PG" }, - "domains/billing/systems/payment-gateway/migration": { + "domains/billing/systems/payments-guide": { title: "Payment Gateway", text: "PG" }, + "domains/billing/systems/payments-guide/migration": { title: "Migration", text: "Migration", }, @@ -371,6 +382,143 @@ describe("RwDocsCollatorFactory", () => { }); }); + describe("duplicate section identities", () => { + const siteRef = "component:default/arch"; + const root = "section:default/root"; + const shared = "system:default/shared"; + const independent = "component:default/independent"; + const deep = "component:default/deep"; + const catalog = () => + createMockCatalog([ + makeEntity("arch", "."), + makeEntity("shared", `${siteRef}#${shared}`, "System"), + ]); + function collisionSite(nested: boolean) { + return createMockSite({ + sections: [ + { sectionRef: root, path: "" }, + { sectionRef: shared, path: "a", ancestors: [root] }, + { sectionRef: shared, path: "a-b", ancestors: [root] }, + { sectionRef: independent, path: "a-b/unique", ancestors: [shared, root] }, + ...(nested + ? [ + { sectionRef: shared, path: "a/nested", ancestors: [shared, root] }, + { sectionRef: deep, path: "a/nested/deep", ancestors: [shared, shared, root] }, + ] + : []), + ], + pages: [ + { sectionRef: root, subpath: "", path: "" }, + { sectionRef: shared, subpath: "", path: "a" }, + { sectionRef: shared, subpath: "", path: "a-b" }, + { sectionRef: shared, subpath: "q", path: "a/q" }, + { sectionRef: shared, subpath: "q", path: "a-b/q" }, + { sectionRef: shared, subpath: "only", path: "a-b/only" }, + { sectionRef: independent, subpath: "p", path: "a-b/unique/p" }, + ...(nested ? [{ sectionRef: deep, subpath: "p", path: "a/nested/deep/p" }] : []), + ], + canonicalRoots: { [shared]: "a" }, + documents: { + "": { title: "Home", text: "Home" }, + a: { title: "Winner", text: "Winner" }, + "a-b": { title: "Loser", text: "Loser" }, + "a/q": { title: "Winner q", text: "Winner body" }, + "a-b/q": { title: "Loser q", text: "Loser body" }, + "a-b/only": { title: "Loser only", text: "Unaddressable" }, + "a-b/unique/p": { title: "Independent p", text: "Independent" }, + "a/nested/deep/p": { title: "Deep p", text: "Deep" }, + }, + }); + } + + it.each([false, true])( + "indexes exact canonical content with valid attribution (nested=%s)", + async (nested) => { + const site = collisionSite(nested); + mockedCreateSite.mockReturnValue(site); + const docs = await collectDocuments(await makeFactory(catalog()).getCollator()); + expect(docs.map((d) => d.title)).not.toEqual( + expect.arrayContaining(["Loser", "Loser q", "Loser only"]), + ); + expect(docs.map((d) => d.title).sort()).toEqual( + ["Home", "Winner", "Winner q", "Independent p", ...(nested ? ["Deep p"] : [])].sort(), + ); + expect(docs.find((d) => d.title === "Winner q")).toMatchObject({ + sectionRef: shared, + subpath: "q", + entityRef: shared, + text: "Winner body", + location: "/catalog/default/system/shared/docs/q", + authorization: { resourceRef: siteRef }, + }); + expect(docs.find((d) => d.title === "Home")).toMatchObject({ + sectionRef: root, + entityRef: siteRef, + location: "/catalog/default/component/arch/docs/", + }); + expect(docs.find((d) => d.title === "Independent p")).toMatchObject({ + sectionRef: independent, + subpath: "p", + entityRef: siteRef, + location: "/catalog/default/component/arch/docs/a-b/unique/p", + }); + expect(docs.filter((d) => d.title === "Deep p")).toMatchObject( + nested + ? [ + { + sectionRef: deep, + subpath: "p", + entityRef: shared, + location: "/catalog/default/system/shared/docs/nested/deep/p", + }, + ] + : [], + ); + expect(docs.every((d) => d.authorization.resourceRef === siteRef)).toBe(true); + expect(site.pagePathFor).toHaveBeenCalledTimes(1); + expect(site.pagePathFor).toHaveBeenCalledWith(shared, ""); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining(siteRef), { + siteRef, + collidingRefs: 1, + omittedSections: nested ? 2 : 1, + omittedPages: 3, + }); + }, + ); + + it.each(["null", "rejected"])( + "emits no partial site documents on collision resolution failure (%s)", + async (failure) => { + const site = collisionSite(true); + if (failure === "null") site.pagePathFor.mockResolvedValue(null); + else site.pagePathFor.mockRejectedValue(new Error("lookup failed")); + mockedCreateSite.mockReturnValueOnce(site); + mockedCreateSite.mockReturnValue( + createFlatSite({ "": { title: "Healthy", text: "Healthy" } }), + ); + const entities = catalog(); + entities.queryEntities.mockResolvedValue({ + items: [makeEntity("arch", "."), makeEntity("healthy", ".")], + pageInfo: {}, + }); + const docs = await collectDocuments(await makeFactory(entities).getCollator()); + expect(docs.map((d) => d.title)).toEqual(["Healthy"]); + expect(site.renderSearchDocument).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining(`Failed to index site ${siteRef}:`), + ); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + failure === "null" + ? `Cannot resolve canonical section root for ${shared}` + : "lookup failed", + ), + ); + }, + ); + }); + it("resolves a section claimed by two entities to the same one every run", async () => { const catalog = createMockCatalog([ makeEntity("zebra", "component:default/arch#domain:default/billing", "Domain"), diff --git a/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.ts b/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.ts index 8e6076f..ce8d313 100644 --- a/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.ts +++ b/plugins/search-backend-module-rw/src/collator/RwDocsCollatorFactory.ts @@ -9,6 +9,7 @@ import { parseEntityRef } from "@backstage/catalog-model"; import { createSite, type RwSite } from "@rwdocs/core"; import { collectSiteClaims, + projectSiteListing, rootClaimOf, toEntityPath, readRwSiteConfig, @@ -148,7 +149,16 @@ export class RwDocsCollatorFactory implements DocumentCollatorFactory { private async *indexSite(claims: SiteClaims): AsyncGenerator { const site = this.createSite(claims.entityPath); - const pages = await site.listPages(); + const [rawSections, rawPages] = await Promise.all([site.listSections(), site.listPages()]); + const { pages, diagnostics } = await projectSiteListing(rawSections, rawPages, (ref) => + site.pagePathFor(ref, ""), + ); + if (diagnostics.collidingRefs > 0) { + this.logger.warn(`Canonical section identity collisions in site ${claims.siteRef}`, { + siteRef: claims.siteRef, + ...diagnostics, + }); + } this.logger.info(`Indexing site ${claims.siteRef} (${pages.length} pages)`); diff --git a/yarn.lock b/yarn.lock index 0e921d0..5b7b771 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4097,17 +4097,17 @@ __metadata: languageName: node linkType: hard -"@fontsource/jetbrains-mono@npm:^5.2.8": - version: 5.2.8 - resolution: "@fontsource/jetbrains-mono@npm:5.2.8" - checksum: 10c0/a51a1ad9cc847ccb3f5986d00b49fcd0eb2061200eb9caf215f968d658806e96e0ee305ab45df3d167f62223e45935cb7e89f984e7fb88fbe6928855a20e044e +"@fontsource/jetbrains-mono@npm:^5.3.0": + version: 5.3.0 + resolution: "@fontsource/jetbrains-mono@npm:5.3.0" + checksum: 10c0/1351b2350e893f5da39559ed90ff0e5e60bcc876b1f40531ad081ad4bf2f3cda3fa9e8940364aef5a7430ae3140685c9cd4819c36d8ad62bcd3fa37e3cc66b1d languageName: node linkType: hard -"@fontsource/roboto@npm:^5.2.10": - version: 5.2.10 - resolution: "@fontsource/roboto@npm:5.2.10" - checksum: 10c0/abadfcaefbe2196bbd36d9258b336063f721894a3624a9639da00792583c565bfd0748ef978f91a38c3c0de8d247615c9be92a763184c085c630fa43f04b1cd9 +"@fontsource/roboto@npm:^5.3.0": + version: 5.3.0 + resolution: "@fontsource/roboto@npm:5.3.0" + checksum: 10c0/68420d2c669ba05f0ea466343151c6d2efff85b05cedbf0f990c58635d86035cae5e7c4904db9abc7b9fd21c5ad179788243ec9dff0c30dfab90679ae2d226f9 languageName: node linkType: hard @@ -7005,7 +7005,7 @@ __metadata: "@backstage/types": "npm:^1.2.2" "@rwdocs/backstage-plugin-rw-common": "workspace:^" "@rwdocs/backstage-plugin-rw-node": "workspace:^" - "@rwdocs/core": "npm:^0.1.35" + "@rwdocs/core": "npm:^0.1.36" "@types/express": "npm:^4.17.0" "@types/jest": "npm:^30.0.0" "@types/supertest": "npm:^7.2.0" @@ -7073,7 +7073,7 @@ __metadata: "@backstage/ui": "npm:^0.17.0" "@material-ui/icons": "npm:^4.11.3" "@rwdocs/backstage-plugin-rw-common": "workspace:^" - "@rwdocs/viewer": "npm:^0.1.35" + "@rwdocs/viewer": "npm:^0.1.36" "@testing-library/dom": "npm:^10.0.0" "@testing-library/jest-dom": "npm:^7.0.0" "@testing-library/react": "npm:^16.0.0" @@ -7114,7 +7114,7 @@ __metadata: "@backstage/plugin-search-backend-node": "npm:^1.4.6" "@backstage/plugin-search-common": "npm:^1.2.24" "@rwdocs/backstage-plugin-rw-common": "workspace:^" - "@rwdocs/core": "npm:^0.1.35" + "@rwdocs/core": "npm:^0.1.36" "@types/jest": "npm:^30.0.0" jest: "npm:^30.2.0" prettier: "npm:^3.4.2" @@ -7126,34 +7126,34 @@ __metadata: languageName: unknown linkType: soft -"@rwdocs/core-darwin-arm64@npm:0.1.35": - version: 0.1.35 - resolution: "@rwdocs/core-darwin-arm64@npm:0.1.35" +"@rwdocs/core-darwin-arm64@npm:0.1.36": + version: 0.1.36 + resolution: "@rwdocs/core-darwin-arm64@npm:0.1.36" conditions: os=darwin & cpu=arm64 languageName: node linkType: hard -"@rwdocs/core-linux-x64-gnu@npm:0.1.35": - version: 0.1.35 - resolution: "@rwdocs/core-linux-x64-gnu@npm:0.1.35" +"@rwdocs/core-linux-x64-gnu@npm:0.1.36": + version: 0.1.36 + resolution: "@rwdocs/core-linux-x64-gnu@npm:0.1.36" conditions: os=linux & cpu=x64 & libc=glibc languageName: node linkType: hard -"@rwdocs/core-linux-x64-musl@npm:0.1.35": - version: 0.1.35 - resolution: "@rwdocs/core-linux-x64-musl@npm:0.1.35" +"@rwdocs/core-linux-x64-musl@npm:0.1.36": + version: 0.1.36 + resolution: "@rwdocs/core-linux-x64-musl@npm:0.1.36" conditions: os=linux & cpu=x64 & libc=musl languageName: node linkType: hard -"@rwdocs/core@npm:^0.1.35": - version: 0.1.35 - resolution: "@rwdocs/core@npm:0.1.35" +"@rwdocs/core@npm:^0.1.36": + version: 0.1.36 + resolution: "@rwdocs/core@npm:0.1.36" dependencies: - "@rwdocs/core-darwin-arm64": "npm:0.1.35" - "@rwdocs/core-linux-x64-gnu": "npm:0.1.35" - "@rwdocs/core-linux-x64-musl": "npm:0.1.35" + "@rwdocs/core-darwin-arm64": "npm:0.1.36" + "@rwdocs/core-linux-x64-gnu": "npm:0.1.36" + "@rwdocs/core-linux-x64-musl": "npm:0.1.36" dependenciesMeta: "@rwdocs/core-darwin-arm64": optional: true @@ -7161,18 +7161,18 @@ __metadata: optional: true "@rwdocs/core-linux-x64-musl": optional: true - checksum: 10c0/8f0183076a833fc24a04b369d4a6abcbf4e1ec6d6d569e9071e37801bbade05482282c7ee1bb690309bcc195e92919aba2765c2690bbf86bf9a398bb97325a7a + checksum: 10c0/31707486913bc8c1b3f751b324869ce3e69953e66a3d0f4b9a59c65b966b2c18f1cb3fdc6ffcc306aedf5347c695b375279afb6de55e121393f1d4abce81ba51 languageName: node linkType: hard -"@rwdocs/viewer@npm:^0.1.35": - version: 0.1.35 - resolution: "@rwdocs/viewer@npm:0.1.35" +"@rwdocs/viewer@npm:^0.1.36": + version: 0.1.36 + resolution: "@rwdocs/viewer@npm:0.1.36" dependencies: - "@fontsource/jetbrains-mono": "npm:^5.2.8" - "@fontsource/roboto": "npm:^5.2.10" + "@fontsource/jetbrains-mono": "npm:^5.3.0" + "@fontsource/roboto": "npm:^5.3.0" approx-string-match: "npm:^2.0.0" - checksum: 10c0/80bf34636056cc8ace8bbcc04f8b3d52ed862d31878282fc8b2d575475eab39b25af6c0d75cd761bccd56ae2c258b2f89d7b6d019a893443013f78c00f39b13a + checksum: 10c0/dea2cd5d2f6ad3232cf58d313d9e2c79a1e844b2ef598cdb4fbe695198d434275d2da0970f08844815affd2c81df7c937679ff0edf337e25959dfc45e990db96 languageName: node linkType: hard