diff --git a/docs/data-model.md b/docs/data-model.md index f5eeaef6..f13430cd 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -1266,6 +1266,13 @@ own pod, as a draft, before publishing it ([deck-library.md](deck-library.md#a-release-outside-the-library)). `assemble` makes a draft a release's text, with every subject moved to the release's address, `@base` that address, and its release time set. + Publishing assembles the draft from the read its checks saw, not from + its documents read again. A draft's version names that read, never + only its documents' ETags: a document read again is a new read unless + the pod answers 304. A server can give no ETag, or the same one for + other content, so ETags alone can't show the draft is unchanged. A + read since, or an edit, makes publishing at that version + `changedElsewhere`. - **Publishing a release** ([solidReleasePublisher.ts](../packages/solid/src/solidReleasePublisher.ts), [studio.md](studio.md#publishing-a-release)) puts that text, as it is, diff --git a/docs/studio.md b/docs/studio.md index fcde4fcb..c8816522 100644 --- a/docs/studio.md +++ b/docs/studio.md @@ -2347,10 +2347,12 @@ releases it, two ways. address is ignored, so such a draft reads as not released, as `data-model.md` says. - **One version.** What is checked, written and marked released is the - draft at the version read (If-Match). A draft changed elsewhere - meanwhile is `changedElsewhere`: before the release is written, - nothing is written; after, the release stays and the draft isn't - marked. + draft at the version read (If-Match). The release is made from the read + the checks saw, not from the documents read again, so it doesn't + rest on the server's ETags, which some servers leave out. A draft + read anew or changed elsewhere meanwhile is `changedElsewhere`: + before the release is written, nothing is written; after, the + release stays and the draft isn't marked. - **Finished again.** Publishing cut short after the release is written (the catalog, the draft or the network failed) is finished by publishing again, at the same address. A document there that states diff --git a/packages/application/src/ports.ts b/packages/application/src/ports.ts index 7b60265a..8c11d5f1 100644 --- a/packages/application/src/ports.ts +++ b/packages/application/src/ports.ts @@ -589,10 +589,12 @@ export interface ReleaseDraftRepository { */ create(instanceUrl: string, draft: ReleaseDraft): Promise; /** - * The draft as its documents say now, with the version they are at - * together (opaque: it changes when any of them does, or one comes or - * goes). Nothing it published before is known here: `published` is - * empty. Throws draftGone when it has no release document, and + * The draft as its documents say now, with the version of this read + * of them (opaque: another when any of them is read anew rather than + * found unchanged, or one comes or goes; never only their ETags, which + * a server may leave out or keep for other content). Nothing it + * published before is known here: `published` is empty. Throws + * draftGone when it has no release document, and * draftIdUnsupported when it names a subject by an id no draft can * have (DRAFT_ID): a chapter's document is named after its id; and * draftHiddenValue when its documents state a predicate a subject's @@ -618,8 +620,10 @@ export interface ReleaseDraftRepository { * (`@base` its URL, the usual prefixes): every subject moved there, * released `issued` (also its time of change), written at library * deck format 6, with the series it starts described in it. With - * `version`, the draft as it was at that version (read), or - * changedElsewhere when its documents changed since. A draft that + * `version`, the draft as the read at that version had it, not read + * again, so what was checked of that read is what is published; or + * changedElsewhere when that read is not the draft's last (read again + * or edited since). A draft that * states anything of `targetUrl` or a subject of it (`#id`) * itself, which would say more of the release than its parts do, is * releaseForeignStatement. One whose documents name a subject of the diff --git a/packages/application/src/releasePublishing.ts b/packages/application/src/releasePublishing.ts index 56c2510a..a3501b84 100644 --- a/packages/application/src/releasePublishing.ts +++ b/packages/application/src/releasePublishing.ts @@ -118,7 +118,8 @@ export function createReleasePublishingUseCases({ return { async publishRelease(draftUrl, targetUrl, markdownCheck) { if (isGuestUrl(draftUrl)) throw new AppError("guestCannotPublish"); - // Read once: what is checked is what is published, and what is marked released. + // Read once: what is checked is what is published (assembled from + // this read, not read again), and what is marked released. const { draft, version } = await drafts.readReleaseDraft(draftUrl); if (draft.root.releasedAs !== undefined) throw new AppError("draftReleased", { url: draft.root.releasedAs }); diff --git a/packages/solid/src/solidReleaseDraftRepository.test.ts b/packages/solid/src/solidReleaseDraftRepository.test.ts index 1befa75b..a78b8197 100644 --- a/packages/solid/src/solidReleaseDraftRepository.test.ts +++ b/packages/solid/src/solidReleaseDraftRepository.test.ts @@ -62,6 +62,25 @@ const RDF = "http://www.w3.org/1999/02/22-rdf-syntax-ns#"; /** The course fixture as a draft of the instance. */ const course = (): ReleaseDraft => rebaseDraft(courseDraft(), DRAFT); +/** + * The pod's reads as a server answers them that gives no ETag (null), or + * always the same one, and ignores If-None-Match: every read a 200. + */ +function withEtag(fetch: typeof globalThis.fetch, etag: string | null): typeof globalThis.fetch { + return (async (input, init) => { + if ((init?.method ?? "GET") !== "GET") return fetch(input, init); + const asked = new Headers(init?.headers); + asked.delete("If-None-Match"); + const response = await fetch(input, { ...init, headers: asked }); + const headers = new Headers(response.headers); + if (etag === null) headers.delete("ETag"); + else headers.set("ETag", etag); + const answer = new Response(await response.text(), { status: response.status, headers }); + Object.defineProperty(answer, "url", { value: response.url || String(input) }); + return answer; + }) as typeof globalThis.fetch; +} + /** The draft with the changes made, which must be. */ function changed(draft: ReleaseDraft, ...changes: DraftChange[]): ReleaseDraft { const result = applyDraftChanges(draft, changes); @@ -999,6 +1018,61 @@ describe("assemble", () => { }); }); + describe.each([ + ["its own ETags", undefined], + ["no ETag", null], + ["the same ETag for other content", '"same"'], + ] as const)("on a pod that gives %s (GHSA-438x-f779-7chm)", (_, etag) => { + const target = `${INSTANCE}releases/solid/v1.ttl`; + const SWAPPED = "Changed after the check"; + + /** A draft read as publishing reads it before its checks, one card changed on the pod since. */ + async function checkedThenChanged() { + const pod = await draftPod(); + await pod.repository.create(INSTANCE, course()); + const repository = createSolidReleaseDraftRepository({ + fetch: etag === undefined ? pod.fetch : withEtag(pod.fetch, etag), + releaseFetch: siteFetch, + }); + const read = await repository.read(DRAFT); + await pod.put(CARDS, (await pod.text(CARDS)).replace('"loose!"@en', `"${SWAPPED}"@en`)); + return { repository, ...read }; + } + + it("makes the release of the draft as read at that version, not as it is now", async () => { + const { repository, version } = await checkedThenChanged(); + const turtle = await repository.assemble(DRAFT, target, NOW, version); + expect(turtle).toContain("loose!"); + expect(turtle).not.toContain(SWAPPED); + expect(await repository.assemble(DRAFT, target, NOW)).toContain(SWAPPED); + }); + + it("refuses to make it, or mark it released, at that version once the draft is read again", async () => { + const { repository, draft, version } = await checkedThenChanged(); + await repository.read(DRAFT); + await expect(repository.assemble(DRAFT, target, NOW, version)).rejects.toMatchObject({ + code: "changedElsewhere", + vars: { url: DRAFT }, + }); + await expect( + repository.applyChanges( + draft, + { ...draft, root: { ...draft.root, releasedAs: target } }, + version, + ), + ).rejects.toMatchObject({ code: "changedElsewhere" }); + }); + }); + + it("keeps the version of a draft read again unchanged, where the pod says so (304)", async () => { + const pod = await draftPod(); + await pod.repository.create(INSTANCE, course()); + const target = `${INSTANCE}releases/solid/v1.ttl`; + const { version } = await pod.repository.read(DRAFT); + expect((await pod.repository.read(DRAFT)).version).toBe(version); + await expect(pod.repository.assemble(DRAFT, target, NOW, version)).resolves.toContain("loose!"); + }); + it("refuses a draft that states anything of the release at its address already, for a pod or the library (GHSA-p2pr-x6fw-hrfx)", async () => { const pod = await draftPod(); const target = `${INSTANCE}releases/solid/v1.ttl`; diff --git a/packages/solid/src/solidReleaseDraftRepository.ts b/packages/solid/src/solidReleaseDraftRepository.ts index 4ba428fd..cee6a2b9 100644 --- a/packages/solid/src/solidReleaseDraftRepository.ts +++ b/packages/solid/src/solidReleaseDraftRepository.ts @@ -54,7 +54,6 @@ import { removeOwned, readDataset, saveDataset, - versionOf, } from "./datasets"; import { draftEntries, @@ -156,11 +155,33 @@ export function createSolidReleaseDraftRepository({ const documentsOf = (draftUrl: string) => draftDocumentsIn(draftUrl, fetch); - /** A draft's documents, read now and remembered as read. */ - async function readDraft(draftUrl: string): Promise<{ draft: ReleaseDraft; version: string }> { + /** A draft's documents, read now. */ + async function readDocuments( + draftUrl: string, + ): Promise<{ urls: readonly string[]; datasets: readonly Dataset[] }> { const urls = await documentsOf(draftUrl); if (!urls.includes(draftUrl)) throw new AppError("draftGone"); - const datasets = await mapAtOnce(urls, (url) => readDataset(url, fetch)); + return { urls, datasets: await mapAtOnce(urls, (url) => readDataset(url, fetch)) }; + } + + /** + * A draft's documents as the read at `version` had them, not read + * again: what was checked of that read is what is published, which a + * server's ETags alone cannot vouch for (GHSA-438x-f779-7chm). + * changedElsewhere when that read is not the draft's last. + */ + function readAt( + draftUrl: string, + version: string, + ): { urls: readonly string[]; datasets: readonly Dataset[] } { + const read = reads.get(draftUrl); + if (read?.version !== version) throw new AppError("changedElsewhere", { url: draftUrl }); + return { urls: [...read.documents.keys()], datasets: [...read.documents.values()] }; + } + + /** A draft's documents, read now and remembered as read. */ + async function readDraft(draftUrl: string): Promise<{ draft: ReleaseDraft; version: string }> { + const { urls, datasets } = await readDocuments(draftUrl); const quads = draftNamedQuads(draftUrl, urls, datasets); // What its documents state otherwise than its records write it would be published, unseen and unchecked (GHSA-32jq-jh7h-r7v4). const draft = draftFromQuads(quads, draftUrl, { refuseHidden: true }); @@ -309,8 +330,8 @@ export function createSolidReleaseDraftRepository({ }, async assemble(draftUrl, targetUrl, issued, version) { - const urls = await documentsOf(draftUrl); - if (!urls.includes(draftUrl)) throw new AppError("draftGone"); + const { urls, datasets } = + version === undefined ? await readDocuments(draftUrl) : readAt(draftUrl, version); const iriOf = draftIriOf(draftContainerOf(draftUrl), draftUrl); const inPlace = inPlaceIn(draftContainerOf(draftUrl), draftUrl); const to = (iri: string) => { @@ -319,9 +340,6 @@ export function createSolidReleaseDraftRepository({ if (isIriPastBound(there)) throw termTooLong(); return there; }; - const datasets = await Promise.all(urls.map((url) => readDataset(url, fetch))); - if (version !== undefined && versionOfDocuments(urls, datasets) !== version) - throw new AppError("changedElsewhere", { url: draftUrl }); // A statement of the release at its address already would say more of it than the draft does, which no editor shows (GHSA-p2pr-x6fw-hrfx). const atTargetAlready = (iri: string) => iri === targetUrl || iri.startsWith(`${targetUrl}#`); const atTarget = datasets.flatMap((dataset, at) => @@ -435,9 +453,29 @@ export function createSolidReleaseDraftRepository({ }; } -/** The version of each of a draft's documents, in order: a draft changed when any of them did, or one came or went. */ +/** The number of each dataset read, given once: see readNumberOf. */ +const READ_NUMBERS = new WeakMap(); +let readsNumbered = 0; + +/** + * A dataset's own number among those read. A document read again is + * another dataset, so another number, unless the pod answered that it + * is unchanged (304), which gives the dataset read before. Its ETag + * alone would not tell two reads apart: a pod may give none, or the + * same one for other content (GHSA-438x-f779-7chm). + */ +function readNumberOf(dataset: object): number { + let number = READ_NUMBERS.get(dataset); + if (number === undefined) { + number = ++readsNumbered; + READ_NUMBERS.set(dataset, number); + } + return number; +} + +/** The version of a read of a draft's documents, in order: another when any of them was read anew, or one came or went. */ function versionOfDocuments(urls: readonly string[], datasets: readonly Dataset[]): string { - return JSON.stringify(urls.map((url, at) => [url, versionOf(datasets[at]!)])); + return JSON.stringify(urls.map((url, at) => [url, readNumberOf(datasets[at]!)])); } /**