diff --git a/docs/validation.md b/docs/validation.md index 14f46684..38055010 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -135,8 +135,11 @@ flowchart LR instance's own folder (the same `data-model.md` section). It's no violation: nothing is blocked or set aside, and the deck is still read and studied from. Those documents aren't - checked, under any spelling of their address (`isSameUrl`), since a - repair writes to the document. Solid Memo's data + checked, under any spelling of their address, since a + repair writes to the document. Each address is compared as a + request sends it (`canonicalUrlOf`), the unclaimed ones brought to + that form once per check, so a catalog of many decks costs one + lookup per document, not a comparison of every pair. Solid Memo's data check and the Studio's Health screen show them (`UnclaimedDecksNotice`) with "These are this deck's documents", which claims them once the user confirms diff --git a/packages/application/src/instanceChecks.test.ts b/packages/application/src/instanceChecks.test.ts index 09a01323..5dd586cd 100644 --- a/packages/application/src/instanceChecks.test.ts +++ b/packages/application/src/instanceChecks.test.ts @@ -11,6 +11,23 @@ import type { LibraryDeckContent } from "@solid-memo/domain/library"; import { instance, deck, libraryContent, makeDeps } from "./testing/fakePorts"; import { current, card2, setup } from "./testing/digestWorld"; +/** How many addresses the code under test brings to the form a request sends (canonicalUrlOf) or compares (isSameUrl). */ +const urlWork = vi.hoisted(() => ({ count: 0 })); +vi.mock("@solid-memo/domain/instanceLayout", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + canonicalUrlOf: (text: string) => { + urlWork.count += 1; + return actual.canonicalUrlOf(text); + }, + isSameUrl: (a: string, b: string) => { + urlWork.count += 2; + return actual.isSameUrl(a, b); + }, + }; +}); + describe("instance checks", () => { it("planRepair plans from a report, and applyRepairs hands the repairs to the repository", async () => { const deps = makeDeps(); @@ -195,6 +212,43 @@ describe("instance checks", () => { expect(checked).toContain(`${instance.url}catalog.ttl`); expect(checked.filter((url) => url.includes("/notes/"))).toEqual([]); }); + it("checks an address that is no URL, unclaimed or not", async () => { + const { deps, useCases } = setup(); + const named = { ...deck, cardsDocumentUrl: "no url" }; + deps.deckRepository.unclaimedDocuments = vi.fn(async () => ["also no url"]); + await useCases.checkDeck(instance.url, named, { plain: (value) => value, check: () => [] }); + expect( + vi.mocked(deps.shapeValidator.validateDocumentSince).mock.calls.map(([url]) => url), + ).toEqual(expect.arrayContaining(["no url", deck.reviewsDocumentUrl])); + }); + it("skips the unclaimed documents of many decks in work that grows with their number, not its square", async () => { + const unclaimedWork = async (count: number) => { + const { deps, useCases } = setup(); + const decks = Array.from({ length: count }, (_, index) => ({ + ...deck, + id: `deck-${index}`, + url: `${instance.url}catalog.ttl#deck-${index}`, + cardsDocumentUrl: `https://elsewhere.example/c${index}.ttl`, + reviewsDocumentUrl: `https://elsewhere.example/r${index}.ttl`, + })); + deps.deckRepository.listDecks = vi.fn(async () => decks); + deps.deckRepository.unclaimedDocuments = vi.fn(async (one) => [ + one.cardsDocumentUrl, + one.reviewsDocumentUrl, + ]); + urlWork.count = 0; + const report = await useCases.checkInstance(instance.url); + const work = urlWork.count; + expect(report.unclaimed).toHaveLength(count); + expect( + vi.mocked(deps.shapeValidator.validateDocumentSince).mock.calls.map(([url]) => url), + ).not.toContain(decks[0]!.cardsDocumentUrl); + return work; + }; + const few = await unclaimedWork(100); + // Ten times the decks, about ten times the work; comparing every pair would be about a hundred. + expect(await unclaimedWork(1000)).toBeLessThan(few * 20); + }); describe("documents outside the instance's folder and any storage of the user's that holds it", () => { const ownElsewhere = "https://alice.example/notes/old-cards.ttl"; const others = "https://bob.example/memo/"; diff --git a/packages/application/src/instanceChecks.ts b/packages/application/src/instanceChecks.ts index e7b24c4d..245d62ad 100644 --- a/packages/application/src/instanceChecks.ts +++ b/packages/application/src/instanceChecks.ts @@ -5,11 +5,11 @@ import { planRepair, type Repair, type RepairPlan } from "@solid-memo/domain/repair"; import type { Deck } from "@solid-memo/domain/deck"; import { + canonicalUrlOf, catalogUrlOf, historyUrlOf, instanceDocumentUrls, isDocumentIn, - isSameUrl, } from "@solid-memo/domain/instanceLayout"; import { summarize, @@ -33,6 +33,7 @@ import type { import type { AnswerStore } from "./answerStore"; import { AppError } from "@solid-memo/domain/appError"; import { documentUrlOf } from "@solid-memo/domain/subjectUrl"; +import { TextMap } from "@solid-memo/domain/pathKeys"; import { mapAtOnce } from "./atOnce"; import { noting } from "./digestStore"; import type { DigestStore } from "./digestStore"; @@ -88,14 +89,32 @@ export interface InstanceCheckUseCases { } /** - * Whether the document at `url` is one of these decks' unclaimed ones, - * in whatever spelling (isSameUrl): a deck may name one document twice, - * and only the first spelling is listed as unclaimed. + * A test of whether the document at an address is one of these decks' + * unclaimed ones, in whatever spelling: a deck may name one document + * twice, and only the first spelling is listed as unclaimed. Each + * address is compared as a request would send it (canonicalUrlOf), the + * unclaimed ones brought to that form once for the test, so that a + * catalog naming many decks costs one lookup per address, not a + * comparison with every unclaimed document. */ -function isUnclaimed(url: string, unclaimed: readonly UnclaimedDeck[]): boolean { - return unclaimed.some((deck) => - deck.documents.some((document) => isSameUrl(documentUrlOf(document), documentUrlOf(url))), - ); +function unclaimedTest(unclaimed: readonly UnclaimedDeck[]): (url: string) => boolean { + // Keyed by pieces (TextMap): an address as a request sends it may be longer than V8 hashes all of. + const skipped = new TextMap(); + for (const deck of unclaimed) { + for (const document of deck.documents) { + const canonical = canonicalDocumentOf(document); + if (canonical !== null) skipped.set(canonical, true); + } + } + return (url) => { + const canonical = canonicalDocumentOf(url); + return canonical !== null && skipped.has(canonical); + }; +} + +/** The address of the document `url` is in, as a request sends it (canonicalUrlOf); null for one that is no URL. */ +function canonicalDocumentOf(url: string): string | null { + return canonicalUrlOf(documentUrlOf(url)); } /** @@ -195,7 +214,7 @@ export function createInstanceCheckUseCases({ /** * The instance's documents, then its drafts': each with where it is, * which picks its shapes. An unclaimed document is not checked, in any - * spelling of its address (isUnclaimed): it may be anyone's, and a + * spelling of its address (unclaimedTest): it may be anyone's, and a * repair of what the check finds writes to it. */ function documentsToCheck( @@ -204,9 +223,10 @@ export function createInstanceCheckUseCases({ drafts: readonly string[], unclaimed: readonly UnclaimedDeck[], ): { url: string; context: Exclude }[] { + const isUnclaimed = unclaimedTest(unclaimed); return [ ...instanceDocumentUrls(instanceUrl, decks) - .filter((url) => !isUnclaimed(url, unclaimed)) + .filter((url) => !isUnclaimed(url)) .map((url) => ({ url, context: "pod" as const })), ...drafts.map((url) => ({ url, context: "draft" as const })), ]; @@ -288,8 +308,9 @@ export function createInstanceCheckUseCases({ const [report, cards, release] = await Promise.all([ Promise.all([digestStore.of(instanceUrl), unclaimedDecksOf(instanceUrl, [deck])]).then( async ([digest, unclaimed]) => { + const isUnclaimed = unclaimedTest(unclaimed); const urls = [deck.cardsDocumentUrl, deck.reviewsDocumentUrl].filter( - (url) => !isUnclaimed(url, unclaimed), + (url) => !isUnclaimed(url), ); return withUnclaimed( await checkDocuments(