fix(solid): publish the draft as its checks read it

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,

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

packages/application/src/ports.ts

@@ -589,10 +589,12 @@ export interface ReleaseDraftRepository {
    */
   create(instanceUrl: string, draft: ReleaseDraft): Promise<ReleaseDraftSummary>;
   /**
-   * 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 (`<targetUrl>#id`)
    * itself, which would say more of the release than its parts do, is
    * releaseForeignStatement. One whose documents name a subject of the

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 });

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`;

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<object, number>();
+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]!)]));
 }
 
 /**