From 4ecfb5f9171480293d67c7377c56d62054781a7b Mon Sep 17 00:00:00 2001 From: Anton Wiklund Date: Sun, 1 Nov 2026 00:13:42 +0100 Subject: [PATCH] fix(solid): read a document's agent names once for all its decks Mapping a deck copied the whole map of the document's agent names, only so that a deck from before format 3 could add the creators it names itself. A catalog or a deck file with many decks and many agents then took time that grows with their product, on every deck-list read of the catalog and on each import of the file. A deck now looks its creators up in a small map of its own names first and in the document's map after, which it no longer copies. Reading a deck file also refuses a file with more than one deck before it reads any of them, as it refused such a file only after mapping them all. Growth tests pin both reads to linear time. Refs: GHSA-hcrw-cr33-jg78 --- docs/testing.md | 2 +- packages/solid/src/mappers/deckMapper.test.ts | 28 ++++++++++++++ packages/solid/src/mappers/deckMapper.ts | 7 ++-- packages/solid/src/solidDeckArchive.test.ts | 38 +++++++++++++++++++ packages/solid/src/solidDeckArchive.ts | 9 +++-- 5 files changed, 76 insertions(+), 8 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 65995e441..44bea09cc 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1022,7 +1022,7 @@ Dependency inversion gives every layer a seam for its fakes: | Layer | Seam | Technique | |---|---|---| -| Domain | none needed | Pure functions; inputs (including `now: Date`) passed as parameters. Plain assertions. What runs over a whole deck or log is timed at two sizes in its own process's CPU time: eight times the input must take under four times as long as an eighth of it done eight times (a linear pass gives about one, work growing with the input about eight). The growth tests (`growth` in `src/testing/growth.ts`, `growthAsync` for work that awaits, `growthAfterSetup` for work that changes what it works on, set up afresh before each run and not timed) time `queryCards` over 5,000 cards, `lapseIndex`, `chainIndexOf` and `statisticsOf` over years of answers, `instanceGraphOf` over an instance's cards and answers, a course's outline (`courseOutlineFromRecords`) over its chapters and steps, the learner's progress in it (`courseProgress`) over a chapter's steps, the release check's course rules (`courseProblems`), a draft's outline (`draftOutline`) over the chapters and steps, the cards the Studio's outline shows (`draftCardFinder`), the deck health's cards that say the same (`duplicateCardsOf`) over many cards alike, a JSON-LD copy (`jsonLdOf` in `solid`) over many values of one property, the search of a Turtle text for its base and prefix declarations (`writeTurtle` in `solid`), the shape check's report on a pod document and its check of a write (`createShaclShapeValidator` in `solid`, over an engine that answers at once) over a catalog's many links that aren't members and many dates it can't read, the notes on a release's making (`makingNotesOf`) over many in one language, a merge of drafts (`mergedDraft`) over many statements of one subject, the blank nodes a removal takes along (`withOnlyTheirBlankNodes` in `solid`) over many of one subject and many subjects naming one, a deck's cards added from a release, removed and brought to this app's format, one card's many distractors added and changed, a deck removed from a cards document another app wrote in, catalog entries and review states brought to this app's format, review states removed, a day's answers removed and many month summaries dropped from the digest, against a pod in memory (`importDeck`, `applyCardChanges`, `removeDeck`, `upgradeCards`, `upgradeDecks`, `upgradeReviewStates`, `applyReviewChanges`, `removeDay` and `updateDigest` in `solid`), a subject's many values set in one call (`setIriAll` and `setStringAll`, in the client modules' contract tests), and the Studio's graph layout and routes over a deck's cards. | +| Domain | none needed | Pure functions; inputs (including `now: Date`) passed as parameters. Plain assertions. What runs over a whole deck or log is timed at two sizes in its own process's CPU time: eight times the input must take under four times as long as an eighth of it done eight times (a linear pass gives about one, work growing with the input about eight). The growth tests (`growth` in `src/testing/growth.ts`, `growthAsync` for work that awaits, `growthAfterSetup` for work that changes what it works on, set up afresh before each run and not timed) time `queryCards` over 5,000 cards, `lapseIndex`, `chainIndexOf` and `statisticsOf` over years of answers, `instanceGraphOf` over an instance's cards and answers, a course's outline (`courseOutlineFromRecords`) over its chapters and steps, the learner's progress in it (`courseProgress`) over a chapter's steps, the release check's course rules (`courseProblems`), a draft's outline (`draftOutline`) over the chapters and steps, the cards the Studio's outline shows (`draftCardFinder`), the deck health's cards that say the same (`duplicateCardsOf`) over many cards alike, a JSON-LD copy (`jsonLdOf` in `solid`) over many values of one property, the search of a Turtle text for its base and prefix declarations (`writeTurtle` in `solid`), the shape check's report on a pod document and its check of a write (`createShaclShapeValidator` in `solid`, over an engine that answers at once) over a catalog's many links that aren't members and many dates it can't read, the notes on a release's making (`makingNotesOf`) over many in one language, a merge of drafts (`mergedDraft`) over many statements of one subject, the blank nodes a removal takes along (`withOnlyTheirBlankNodes` in `solid`) over many of one subject and many subjects naming one, a catalog's decks (`toDecks` in `solid`) and a deck file's refusal (`readDeckFile`) over many decks and agents, a deck's cards added from a release, removed and brought to this app's format, one card's many distractors added and changed, a deck removed from a cards document another app wrote in, catalog entries and review states brought to this app's format, review states removed, a day's answers removed and many month summaries dropped from the digest, against a pod in memory (`importDeck`, `applyCardChanges`, `removeDeck`, `upgradeCards`, `upgradeDecks`, `upgradeReviewStates`, `applyReviewChanges`, `removeDay` and `updateDigest` in `solid`), a subject's many values set in one call (`setIriAll` and `setStringAll`, in the client modules' contract tests), and the Studio's graph layout and routes over a deck's cards. | | Application | ports | Fake ports are injected (`vi.fn` per method): the fixtures and fake ports in [fakePorts.ts](../packages/application/src/testing/fakePorts.ts), and `NO_PORTS` ([noPorts.ts](../packages/application/src/testing/noPorts.ts)), which other packages' tests spread too, for the ports a test doesn't use. Each slice is tested in its own `.test.ts`, the stores the slices share in theirs. No module mocking. | | UI | `UseCases` prop | Screens render with a fake `UseCases`: `makeUseCasesFake` ([useCasesFake.ts](../packages/ui/src/test/useCasesFake.ts)) makes every slice's fake ([test/fakes/](../packages/ui/src/test/fakes/)) in one object, each calling the others through it. A screen that takes a slice (the trial's) can be given that slice's fake alone. Assertions use testing-library queries. Query-dependent components get a fresh `QueryClient` (retries off). | | Infrastructure mappers | none needed | Pure `Dataset`/`Subject` → domain functions, fed in-memory datasets built with `datasetFrom`/`buildThing` ([datasets.ts](../packages/solid/src/testing/datasets.ts)). | diff --git a/packages/solid/src/mappers/deckMapper.test.ts b/packages/solid/src/mappers/deckMapper.test.ts index 9ec6b159e..8fe166726 100644 --- a/packages/solid/src/mappers/deckMapper.test.ts +++ b/packages/solid/src/mappers/deckMapper.test.ts @@ -15,9 +15,11 @@ import { type NamedSubject, removeThing, setThing, + setThings, type SubjectBuilder, } from "../client/rdf"; import { datasetFrom } from "../testing/datasets"; +import { growth } from "@solid-memo/domain/testing/growth"; import { deckSubjects, toCard, @@ -373,6 +375,32 @@ describe("toDeck", () => { }); }); +describe("toDecks over many decks and agents", () => { + it("takes time that grows with them, not their product (GHSA-hcrw-cr33-jg78)", () => { + const ratio = growth( + (count) => { + const subjects = Array.from({ length: count }, (_, i) => [ + buildThing(createThing({ url: `${CATALOG}#deck-${i}` })) + .addIri(RDF.type, SM.Deck) + .addStringNoLocale(DCTERMS.title, "t") + .addIri(SM.cardsDocument, CARDS_DOC) + .addIri(SM.reviewsDocument, REVIEWS_DOC) + .build(), + buildThing(createThing({ url: `${CATALOG}#agent-${i}` })) + .addIri(RDF.type, "http://xmlns.com/foaf/0.1/Agent") + .addStringNoLocale("http://xmlns.com/foaf/0.1/name", `n${i}`) + .build(), + ]).flat(); + const catalog = setThings(datasetFrom(CATALOG), subjects); + return () => toDecks(catalog, CATALOG); + }, + 1000, + 8, + ); + expect(ratio).toBeLessThan(4); + }); +}); + describe("toCard", () => { /** The card a subject is, alone in its document. */ const cardOf = (thing: NamedSubject) => toCard(thing, datasetFrom(CARDS_DOC)); diff --git a/packages/solid/src/mappers/deckMapper.ts b/packages/solid/src/mappers/deckMapper.ts index 3917c0c34..9628db647 100644 --- a/packages/solid/src/mappers/deckMapper.ts +++ b/packages/solid/src/mappers/deckMapper.ts @@ -117,9 +117,10 @@ export function toDeck( const read = readVersioned(thing, "deck"); if (read === null) return null; const url = asUrl(thing); - const names = new Map(agentNames); + // Only a deck before format 3 names agents of its own: the document's are not copied for each deck, which made many decks and agents take quadratic time (GHSA-hcrw-cr33-jg78). + const own = new Map(); if (read.record.version < 3) { - for (const author of read.record.data.creator) names.set(agentUrlOf(url, author), author); + for (const author of read.record.data.creator) own.set(agentUrlOf(url, author), author); } const completedChapters = getUrlAll(thing, SM.completedChapter); const storedLanguage = getStringNoLocale(thing, SM.deckCardLanguage); @@ -129,7 +130,7 @@ export function toDeck( url, read.storedVersion, migrate("deck", read.record, { subject: url }), - (agent) => names.get(agent) ?? agent, + (agent) => own.get(agent) ?? agentNames.get(agent) ?? agent, ), // Outside the deck's shape, like sm:position: every write of the entry keeps them (recordThing). ...(getUrlAll(thing, RDF.type).includes(SCHEMA.Course) ? { course: true as const } : {}), diff --git a/packages/solid/src/solidDeckArchive.test.ts b/packages/solid/src/solidDeckArchive.test.ts index 5cab985b1..345d69042 100644 --- a/packages/solid/src/solidDeckArchive.test.ts +++ b/packages/solid/src/solidDeckArchive.test.ts @@ -6,14 +6,22 @@ import { describe, expect, it, vi } from "vitest"; import { CARD_FORMAT_VERSION, type Card, type Deck } from "@solid-memo/domain/deck"; import { DECK_FILE_BASE, type DeckFileFormat, importedDeck } from "@solid-memo/domain/deckFile"; import type { FsrsState, ReviewState } from "@solid-memo/domain/review"; +import { growthAsync } from "@solid-memo/domain/testing/growth"; import { LATEST_VERSION } from "@solid-memo/vocab/types.generated"; import { RELEASE_LIMITS } from "./client/readLimits"; +import { agentNamesOf, toDeck } from "./mappers/deckMapper"; import { createSolidDeckArchive, parseDeckFile } from "./solidDeckArchive"; import { createSolidDeckRepository } from "./solidDeckRepository"; import { createSolidReviewStateRepository } from "./solidReviewStateRepository"; import { fakePod } from "./testing/fakePod"; import { SCHEMA, SM, SM_NS } from "./vocab"; +// The real mappers, watched: a test checks that a file of several decks maps none of them. +vi.mock("./mappers/deckMapper", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, agentNamesOf: vi.fn(actual.agentNamesOf), toDeck: vi.fn(actual.toDeck) }; +}); + const INSTANCE = "https://pod.example/solid-memo/a/"; const FOREIGN = "https://other.example/ns#rating"; const CHAPTER = "https://solid-memo.com/decks/solid/v1.ttl#chapter-1"; @@ -303,6 +311,36 @@ describe("readDeckFile", () => { }); }); + it("refuses a file of more than one deck before it maps any (GHSA-hcrw-cr33-jg78)", async () => { + vi.mocked(toDeck).mockClear(); + vi.mocked(agentNamesOf).mockClear(); + const two = `${OLD_DECK}<#other> a sm:Deck ; dcterms:title "B" ; sm:cardsDocument ; sm:reviewsDocument .`; + await expect(archive.readDeckFile(two, "turtle", base)).rejects.toMatchObject({ + code: "notADeckFile", + }); + expect(toDeck).not.toHaveBeenCalled(); + expect(agentNamesOf).not.toHaveBeenCalled(); + }); + + it("refuses a file of many decks and agents in time that grows with them, not their product (GHSA-hcrw-cr33-jg78)", async () => { + const ratio = await growthAsync( + (count) => { + let text = PREFIXES; + for (let i = 0; i < count; i++) { + text += `<#d${i}> a sm:Deck ; dcterms:title "t" ; sm:cardsDocument ; sm:reviewsDocument .\n`; + text += `<#a${i}> a ; "n${i}" .\n`; + } + return () => + expect(archive.readDeckFile(text, "turtle", base)).rejects.toMatchObject({ + code: "notADeckFile", + }); + }, + 1000, + 8, + ); + expect(ratio).toBeLessThan(4); + }); + it("refuses text that does not parse", async () => { await expect(archive.readDeckFile("<#a> <#b>", "turtle", base)).rejects.toMatchObject({ code: "deckFileUnreadable", diff --git a/packages/solid/src/solidDeckArchive.ts b/packages/solid/src/solidDeckArchive.ts index 4995b5455..9cdbff81f 100644 --- a/packages/solid/src/solidDeckArchive.ts +++ b/packages/solid/src/solidDeckArchive.ts @@ -106,10 +106,11 @@ export function createSolidDeckArchive({ fetch }: { fetch: typeof globalThis.fet for (const thing of things) refuseNewer(thing); const typed = (type: string) => things.filter((thing) => getUrlAll(thing, RDF.type).includes(type)); - const names = agentNamesOf(dataset); - const decks = typed(SM.Deck).map((thing) => toDeck(thing, names)); - if (decks.length !== 1 || decks[0] === null) throw new AppError("notADeckFile"); - const deck = decks[0]!; + // A file of more than one deck is refused before any is read (GHSA-hcrw-cr33-jg78). + const deckThings = typed(SM.Deck); + if (deckThings.length !== 1) throw new AppError("notADeckFile"); + const deck = toDeck(deckThings[0]!, agentNamesOf(dataset)); + if (deck === null) throw new AppError("notADeckFile"); const upgraded: FormatUpgrade[] = []; const dropped: string[] = [];