From e1292ad678ed12cb2886ab4be7a4699285c98489 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Fri, 4 Sep 2026 07:31:56 -0400 Subject: [PATCH 1/5] ENG-1870-Copy-Roam-assets-in-supabase-Task-2.1 --- .../__tests__/findAssetReferences.test.ts | 101 ++++++++++++++++++ apps/roam/src/utils/findAssetReferences.ts | 72 +++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 apps/roam/src/utils/__tests__/findAssetReferences.test.ts create mode 100644 apps/roam/src/utils/findAssetReferences.ts diff --git a/apps/roam/src/utils/__tests__/findAssetReferences.test.ts b/apps/roam/src/utils/__tests__/findAssetReferences.test.ts new file mode 100644 index 000000000..303da29ed --- /dev/null +++ b/apps/roam/src/utils/__tests__/findAssetReferences.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; +import { findAssetReferences, isRoamStorageUrl } from "../findAssetReferences"; + +const roamAsset = (name: string) => + `https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2F${name}?alt=media&token=9f1c07a4-2b3e-4c5d-8a91-6e0f2d7b4c13`; + +const IMAGE = roamAsset("lqP2ioVNC3.png"); +const PDF = roamAsset("GVfB6XBcMR.pdf"); +const AUDIO = roamAsset("Kd8xTbQ1Ln.mp3"); +const VIDEO = roamAsset("Zr4mWpN70c.mp4"); +const DOCX = roamAsset("Vx9sLcE22a.docx"); + +describe("findAssetReferences", () => { + it("finds an image embed", () => { + expect(findAssetReferences(`Some text\n\n![](${IMAGE})\n`)).toEqual([ + IMAGE, + ]); + expect(findAssetReferences(`![a diagram](${IMAGE})`)).toEqual([IMAGE]); + }); + + it("finds the non-image embeds Roam writes", () => { + expect(findAssetReferences(`{{[[pdf]]: ${PDF}}}`)).toEqual([PDF]); + expect(findAssetReferences(`{{[[audio]]: ${AUDIO}}}`)).toEqual([AUDIO]); + expect(findAssetReferences(`{{[[video]]: ${VIDEO}}}`)).toEqual([VIDEO]); + }); + + it("finds a non-image embed written without the page brackets", () => { + expect(findAssetReferences(`{{pdf: ${PDF}}}`)).toEqual([PDF]); + }); + + it("finds a bare URL", () => { + expect(findAssetReferences(`Protocol: ${DOCX}`)).toEqual([DOCX]); + }); + + it("finds a file linked rather than embedded", () => { + expect(findAssetReferences(`See [the protocol](${DOCX}).`)).toEqual([DOCX]); + }); + + it("ignores an external link", () => { + const markdown = [ + "See [Rasch & Born 2013](https://www.science.org/doi/10.1126/science.1234567).", + "![hotlinked](https://example.com/someone-elses.png)", + "https://roamresearch.com/#/app/MAPLab/page/tgWb6JozF", + ].join("\n"); + expect(findAssetReferences(markdown)).toEqual([]); + }); + + it("keeps Roam-hosted assets while ignoring external links around them", () => { + const markdown = [ + `![](${IMAGE})`, + "[a paper](https://example.com/paper.pdf)", + `{{[[pdf]]: ${PDF}}}`, + ].join("\n\n"); + expect(findAssetReferences(markdown)).toEqual([IMAGE, PDF]); + }); + + it("counts an asset once however often it appears, in order of first appearance", () => { + const markdown = `![](${PDF})\n\n![](${IMAGE})\n\n{{[[pdf]]: ${PDF}}}\n\n${IMAGE}`; + expect(findAssetReferences(markdown)).toEqual([PDF, IMAGE]); + }); + + it("does not double count a URL that sits inside an embed", () => { + expect(findAssetReferences(`![](${IMAGE})`)).toHaveLength(1); + }); + + it("drops sentence punctuation that follows a bare URL", () => { + expect(findAssetReferences(`The protocol is at ${DOCX}.`)).toEqual([DOCX]); + }); + + it("returns nothing for content with no assets", () => { + expect(findAssetReferences("# A title\n\nJust prose.\n")).toEqual([]); + expect(findAssetReferences("")).toEqual([]); + }); +}); + +describe("isRoamStorageUrl", () => { + it("accepts a Roam upload", () => { + expect(isRoamStorageUrl(IMAGE)).toBe(true); + }); + + it("rejects another Firebase project on the same host", () => { + expect( + isRoamStorageUrl( + "https://firebasestorage.googleapis.com/v0/b/someone-else.appspot.com/o/imgs%2Fx.png?alt=media", + ), + ).toBe(false); + }); + + it("rejects a lookalike host", () => { + expect( + isRoamStorageUrl( + "https://firebasestorage.googleapis.com.evil.test/v0/b/firescript-577a2.appspot.com/o/x.png", + ), + ).toBe(false); + }); + + it("rejects anything that is not a URL", () => { + expect(isRoamStorageUrl("attachments/figure.png")).toBe(false); + expect(isRoamStorageUrl("")).toBe(false); + }); +}); diff --git a/apps/roam/src/utils/findAssetReferences.ts b/apps/roam/src/utils/findAssetReferences.ts new file mode 100644 index 000000000..a1a3826fb --- /dev/null +++ b/apps/roam/src/utils/findAssetReferences.ts @@ -0,0 +1,72 @@ +/** + * Finds the assets a node's `full` markdown references, so the publisher can copy their + * bytes into shared storage. + * + * Roam addresses assets by URL, and the markdown is never rewritten at publication. What + * this returns is therefore exactly what lands in `FileReference.filepath` and in + * `CrossAppAsset.sourceRef`. + */ + +/** Roam uploads land in its own Firebase project, which we do not control. */ +const ROAM_STORAGE_HOST = "firebasestorage.googleapis.com"; +const ROAM_STORAGE_BUCKET = "firescript-577a2.appspot.com"; + +const URL_PATTERN = String.raw`https?://[^\s<>()\[\]{}"']+`; + +/** + * The forms Roam writes an uploaded file in. `file.upload` returns a media-type-dependent + * block string, so one asset can appear as any of these; a plain markdown link covers a + * file a user linked rather than embedded. + * + * Ordered, and matched in one pass, so a URL inside an embed is never also counted as a + * bare URL. Every branch captures the URL, and exactly one group is defined per match. + */ +const ASSET_REFERENCE_PATTERN = new RegExp( + [ + String.raw`!\[[^\]]*\]\((${URL_PATTERN})\)`, // ![](url) image embed + String.raw`\{\{\[\[(?:pdf|audio|video)\]\]:\s*(${URL_PATTERN})\s*\}\}`, // {{[[pdf]]: url}} + String.raw`\{\{(?:pdf|audio|video):\s*(${URL_PATTERN})\s*\}\}`, // {{pdf: url}} + String.raw`\[[^\]]*\]\((${URL_PATTERN})\)`, // [label](url) plain link + `(${URL_PATTERN})`, // a bare URL, which Roam renders as a link + ].join("|"), + "g", +); + +/** Punctuation that ends a sentence rather than the URL it follows. */ +const TRAILING_PUNCTUATION = /[.,;:!?]+$/; + +/** + * Whether a URL points at Roam's own storage, and so at bytes this graph can fetch and + * publish. Anything else (an image hotlinked from another site, a link to a paper) is a + * resource that was never an asset of the node, and is left alone. + * + * Matching the bucket rather than the path shape: every Roam upload lands in this one + * bucket, while the path (`imgs/app//`) is Roam's to change. + */ +export const isRoamStorageUrl = (url: string): boolean => { + try { + const { hostname, pathname } = new URL(url); + return ( + hostname === ROAM_STORAGE_HOST && + pathname.includes(`/${ROAM_STORAGE_BUCKET}/`) + ); + } catch { + return false; + } +}; + +/** + * The Roam-hosted assets referenced by `markdown`, deduplicated, in order of first + * appearance. The same asset embedded twice is one asset to copy, and one row to write. + */ +export const findAssetReferences = (markdown: string): string[] => { + const found = new Set(); + for (const match of markdown.matchAll(ASSET_REFERENCE_PATTERN)) { + const [, ...groups] = match; + const captured = groups.find((group) => group !== undefined); + if (captured === undefined) continue; + const url = captured.replace(TRAILING_PUNCTUATION, ""); + if (isRoamStorageUrl(url)) found.add(url); + } + return [...found]; +}; From 0337dab29231c6f5b2fbc118129fc7cda0266ff7 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Fri, 4 Sep 2026 07:32:14 -0400 Subject: [PATCH 2/5] ENG-1870-Copy-Roam-assets-in-supabase-Task-2.2 --- .../utils/__tests__/fetchRoamAsset.test.ts | 180 ++++++++++++++++++ apps/roam/src/utils/fetchRoamAsset.ts | 131 +++++++++++++ 2 files changed, 311 insertions(+) create mode 100644 apps/roam/src/utils/__tests__/fetchRoamAsset.test.ts create mode 100644 apps/roam/src/utils/fetchRoamAsset.ts diff --git a/apps/roam/src/utils/__tests__/fetchRoamAsset.test.ts b/apps/roam/src/utils/__tests__/fetchRoamAsset.test.ts new file mode 100644 index 000000000..507ff8487 --- /dev/null +++ b/apps/roam/src/utils/__tests__/fetchRoamAsset.test.ts @@ -0,0 +1,180 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + assetDescriptorUrl, + fetchAssetBytes, + fetchAssetDescriptor, +} from "../fetchRoamAsset"; + +const ASSET_URL = + "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FlqP2ioVNC3.png?alt=media&token=9f1c07a4-2b3e-4c5d-8a91-6e0f2d7b4c13"; + +/** A descriptor as Firebase returns it, with Roam's custom metadata. */ +const descriptorWithName = { + name: "imgs/app/MAPLab/lqP2ioVNC3.png", + contentType: "image/png", + size: "20480", + md5Hash: "2+a5zmgB4cXuTfCAbtPGJQ==", + metadata: { + "file-type": "image/png", + "file-name": "CleanShot 2025-11-16 at 17.14.44@2x.png", + }, +}; + +const descriptorWithoutName = { + name: "imgs/app/MAPLab/GVfB6XBcMR.pdf", + contentType: "application/pdf", + size: "51200", +}; + +/** Answers the descriptor request with JSON and the bytes request with a body. */ +const mockFetch = ({ + descriptor, + bytes = "PNGDATA", + descriptorStatus = 200, + bytesStatus = 200, +}: { + descriptor: unknown; + bytes?: string; + descriptorStatus?: number; + bytesStatus?: number; +}) => { + const calls: string[] = []; + const fetchMock = vi.fn((input: string) => { + calls.push(input); + const isDescriptor = !input.includes("alt=media"); + if (isDescriptor) + return Promise.resolve({ + ok: descriptorStatus === 200, + status: descriptorStatus, + json: () => Promise.resolve(descriptor), + } as unknown as Response); + return Promise.resolve({ + ok: bytesStatus === 200, + status: bytesStatus, + arrayBuffer: () => + Promise.resolve(new TextEncoder().encode(bytes).buffer), + } as unknown as Response); + }); + vi.stubGlobal("fetch", fetchMock); + return { calls, fetchMock }; +}; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("assetDescriptorUrl", () => { + it("removes alt=media so the request returns the descriptor", () => { + expect(assetDescriptorUrl(ASSET_URL)).not.toContain("alt=media"); + }); + + it("keeps the download token, which governs access to both", () => { + expect(assetDescriptorUrl(ASSET_URL)).toContain( + "token=9f1c07a4-2b3e-4c5d-8a91-6e0f2d7b4c13", + ); + }); +}); + +describe("fetchAssetDescriptor", () => { + it("resolves the uploaded name when Roam recorded one", async () => { + mockFetch({ descriptor: descriptorWithName }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toEqual({ + filename: "CleanShot 2025-11-16 at 17.14.44@2x.png", + mimetype: "image/png", + size: 20480, + }); + }); + + it("falls back to the storage uid when the name key is absent", async () => { + mockFetch({ descriptor: descriptorWithoutName }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toEqual({ + filename: "GVfB6XBcMR.pdf", + mimetype: "application/pdf", + size: 51200, + }); + }); + + it("falls back to the storage uid when there is no metadata at all", async () => { + mockFetch({ descriptor: { name: "imgs/app/MAPLab/lqP2ioVNC3.png" } }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toMatchObject({ + filename: "lqP2ioVNC3.png", + mimetype: "application/octet-stream", + }); + }); + + it("falls back to the uid in the URL when the descriptor names no object", async () => { + mockFetch({ descriptor: { contentType: "image/png" } }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toMatchObject({ + filename: "lqP2ioVNC3.png", + }); + }); + + it("reports no size when the descriptor sends null rather than omitting it", async () => { + mockFetch({ descriptor: { ...descriptorWithoutName, size: null } }); + + // Not 0: `Number(null)` would read as an empty file and wave the asset past the + // pre-download cap check. + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toMatchObject({ + size: undefined, + }); + }); + + it("reports no size when the descriptor sends an empty string", async () => { + mockFetch({ descriptor: { ...descriptorWithoutName, size: "" } }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toMatchObject({ + size: undefined, + }); + }); + + it("reports no size when the descriptor does not give one", async () => { + mockFetch({ descriptor: { name: "imgs/app/MAPLab/lqP2ioVNC3.png" } }); + + await expect(fetchAssetDescriptor(ASSET_URL)).resolves.toMatchObject({ + size: undefined, + }); + }); + + it("does not transfer the bytes", async () => { + const { calls } = mockFetch({ descriptor: descriptorWithName }); + + await fetchAssetDescriptor(ASSET_URL); + + expect(calls).toHaveLength(1); + expect(calls[0]).not.toContain("alt=media"); + }); + + it("throws when the descriptor cannot be read", async () => { + mockFetch({ descriptor: {}, descriptorStatus: 404 }); + + await expect(fetchAssetDescriptor(ASSET_URL)).rejects.toThrow( + /Could not read asset descriptor \(404\)/, + ); + }); +}); + +describe("fetchAssetBytes", () => { + it("downloads the bytes, without reading the descriptor", async () => { + const { calls } = mockFetch({ + descriptor: descriptorWithName, + bytes: "PNGDATA", + }); + + const content = await fetchAssetBytes(ASSET_URL); + + expect(new TextDecoder().decode(content)).toBe("PNGDATA"); + expect(calls).toEqual([ASSET_URL]); + }); + + it("throws when the bytes cannot be fetched", async () => { + mockFetch({ descriptor: descriptorWithName, bytesStatus: 403 }); + + await expect(fetchAssetBytes(ASSET_URL)).rejects.toThrow( + /Could not fetch asset \(403\)/, + ); + }); +}); diff --git a/apps/roam/src/utils/fetchRoamAsset.ts b/apps/roam/src/utils/fetchRoamAsset.ts new file mode 100644 index 000000000..6f6f7a80a --- /dev/null +++ b/apps/roam/src/utils/fetchRoamAsset.ts @@ -0,0 +1,131 @@ +/** + * Fetches one asset from Roam's storage, along with the name it was uploaded under. + * + * Roam's storage URL carries a random uid rather than the original file name + * (`imgs/app//lqP2ioVNC3.png`), so the name has to be read separately. Firebase + * keeps it in the object's custom metadata, and requesting the same URL with `alt=media` + * removed returns the object descriptor as JSON instead of the bytes. That read is cheap, + * needs no Roam API, and also yields the size, so a caller can decide whether an asset is + * worth downloading before downloading it. + */ + +/** Where Roam records the uploaded name, alongside `file-type`. */ +const UPLOADED_NAME_KEY = "file-name"; + +type FirebaseObjectDescriptor = { + /** The full object path, e.g. "imgs/app/MAPLab/lqP2ioVNC3.png". */ + name?: string; + contentType?: string; + /** Firebase reports the byte count as a string, and may omit it or send null. */ + size?: string | null; + timeCreated?: string; + updated?: string; + metadata?: Record; +}; + +export type RoamAssetDescriptor = { + /** The name the file was uploaded under, or the storage uid when Roam kept none. */ + filename: string; + mimetype: string; + /** Byte count, or undefined when the descriptor did not report one. */ + size: number | undefined; + /** When Roam's storage recorded the object, where it reports them. */ + createdAt: Date | undefined; + modifiedAt: Date | undefined; +}; + +const DEFAULT_MIMETYPE = "application/octet-stream"; + +/** + * The same URL with `alt=media` removed, which returns the object descriptor rather than + * the bytes. The download token is kept: it governs access to both. + */ +export const assetDescriptorUrl = (assetUrl: string): string => { + const url = new URL(assetUrl); + url.searchParams.delete("alt"); + return url.toString(); +}; + +/** + * The storage uid, used as the name when Roam recorded none. This is what Roam's own + * `file.get` falls back to, so an asset with no metadata gets the same name here as it + * would there. + */ +const storageUidFromPath = (objectPath: string): string => { + const segments = decodeURIComponent(objectPath).split("/"); + return segments[segments.length - 1] ?? ""; +}; + +const storageUidFromUrl = (assetUrl: string): string => { + try { + return storageUidFromPath(new URL(assetUrl).pathname); + } catch { + return ""; + } +}; + +const parseTimestamp = (value: string | undefined): Date | undefined => { + if (value === undefined) return undefined; + const parsed = new Date(value); + return Number.isNaN(parsed.valueOf()) ? undefined : parsed; +}; + +const describeAsset = ( + descriptor: FirebaseObjectDescriptor, + assetUrl: string, +): RoamAssetDescriptor => { + const uploadedName = descriptor.metadata?.[UPLOADED_NAME_KEY]; + const fallbackName = + descriptor.name !== undefined + ? storageUidFromPath(descriptor.name) + : storageUidFromUrl(assetUrl); + // Only a non-empty string is a size. `Number(null)` and `Number("")` are both 0, which + // would read as "empty file" and wave an unmeasured asset past the pre-download cap. + const rawSize = descriptor.size; + const size = + typeof rawSize === "string" && rawSize.trim() !== "" + ? Number(rawSize) + : NaN; + return { + filename: uploadedName || fallbackName, + mimetype: descriptor.contentType || DEFAULT_MIMETYPE, + size: Number.isFinite(size) ? size : undefined, + createdAt: parseTimestamp(descriptor.timeCreated), + modifiedAt: parseTimestamp(descriptor.updated), + }; +}; + +/** + * Reads the object descriptor without transferring the file. + * + * Throws when the read fails. The publish stage catches per asset, so one unreadable + * asset never fails its node. + */ +export const fetchAssetDescriptor = async ( + assetUrl: string, +): Promise => { + const response = await fetch(assetDescriptorUrl(assetUrl)); + if (!response.ok) + throw new Error( + `Could not read asset descriptor (${response.status}): ${assetUrl}`, + ); + const descriptor = (await response.json()) as FirebaseObjectDescriptor; + return describeAsset(descriptor, assetUrl); +}; + +/** + * Downloads the bytes alone. + * + * Deliberately kept separate from `fetchAssetDescriptor` rather than composed with it: + * the size cap is checked from the descriptor first, so an oversized asset's bytes never + * cross the network. A function that read the descriptor and downloaded in one step would + * foreclose that. Throws when the request fails. + */ +export const fetchAssetBytes = async ( + assetUrl: string, +): Promise => { + const response = await fetch(assetUrl); + if (!response.ok) + throw new Error(`Could not fetch asset (${response.status}): ${assetUrl}`); + return response.arrayBuffer(); +}; From c88e7ff1dd624ff5e315ff7bcb7465a060211d3b Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Fri, 4 Sep 2026 07:38:55 -0400 Subject: [PATCH 3/5] ENG-1870-Copy-Roam-assets-in-supabase-Task-2.3 --- .../copyAssetToSharedStorage.test.ts | 196 ++++++++++++++++++ .../src/utils/copyAssetToSharedStorage.ts | 101 +++++++++ .../database/src/lib/__tests__/files.test.ts | 9 + packages/database/src/lib/files.ts | 4 +- 4 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 apps/roam/src/utils/__tests__/copyAssetToSharedStorage.test.ts create mode 100644 apps/roam/src/utils/copyAssetToSharedStorage.ts diff --git a/apps/roam/src/utils/__tests__/copyAssetToSharedStorage.test.ts b/apps/roam/src/utils/__tests__/copyAssetToSharedStorage.test.ts new file mode 100644 index 000000000..4baffa8fa --- /dev/null +++ b/apps/roam/src/utils/__tests__/copyAssetToSharedStorage.test.ts @@ -0,0 +1,196 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import { MAX_PUBLISHED_ASSET_BYTES } from "@repo/database/lib/assetLimits"; +import { copyAssetToSharedStorage } from "../copyAssetToSharedStorage"; + +const asset = (uid: string) => + `https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2F${uid}?alt=media&token=9f1c07a4`; + +const IMAGE = asset("lqP2ioVNC3.png"); +const OTHER = asset("Zr4mWpN70c.png"); + +type Row = { filepath?: unknown; filehash?: unknown; source_path?: unknown }; + +/** + * A stand-in for Supabase that behaves like the real one where this code depends on it: + * `file_exists` answers from the rows already written, and the primary key rejects a + * repeated reference. + */ +const makeClient = () => { + const rows = new Map(); + const upload = vi.fn().mockResolvedValue({ error: null }); + const thenable = (result: unknown) => ({ + then: (resolve: (value: unknown) => unknown) => + Promise.resolve(result).then(resolve), + }); + const client = { + rpc: vi.fn((_fn: string, { hashvalue }: { hashvalue: string }) => + Promise.resolve({ + data: [...rows.values()].some((row) => row.filehash === hashvalue), + error: null, + }), + ), + storage: { from: vi.fn(() => ({ upload })) }, + from: vi.fn(() => ({ + insert: vi.fn((row: Row) => { + if (rows.has(String(row.filepath))) + return thenable({ error: { code: "23505", message: "duplicate" } }); + rows.set(String(row.filepath), { ...row }); + return thenable({ error: null }); + }), + update: vi.fn(() => { + const builder = { + eq: vi.fn(() => builder), + then: thenable({ error: null }).then, + }; + return builder; + }), + })), + } as unknown as DGSupabaseClient; + return { client, rows, upload }; +}; + +const descriptorFor = ({ + size, + name = "CleanShot 2025-11-16 at 17.14.44@2x.png", +}: { + size?: number; + name?: string; +}) => ({ + name: "imgs/app/MAPLab/lqP2ioVNC3.png", + contentType: "image/png", + ...(size === undefined ? {} : { size: String(size) }), + timeCreated: "2026-06-12T14:00:00.000Z", + updated: "2026-06-12T15:00:00.000Z", + metadata: { "file-name": name }, +}); + +const mockFetch = ({ + descriptor, + bytes, +}: { + descriptor: unknown; + bytes: string; +}) => { + const calls: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn((input: string) => { + calls.push(input); + if (!input.includes("alt=media")) + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve(descriptor), + } as unknown as Response); + return Promise.resolve({ + ok: true, + status: 200, + arrayBuffer: () => + Promise.resolve(new TextEncoder().encode(bytes).buffer), + } as unknown as Response); + }), + ); + return { calls }; +}; + +const copy = ( + client: DGSupabaseClient, + assetUrl = IMAGE, +): ReturnType => + copyAssetToSharedStorage({ + client, + spaceId: 20, + sourceLocalId: "node-1", + assetUrl, + nodeCreated: new Date("2026-06-01T00:00:00.000Z"), + nodeLastModified: new Date("2026-06-02T00:00:00.000Z"), + }); + +describe("copyAssetToSharedStorage", () => { + let harness: ReturnType; + + beforeEach(() => { + harness = makeClient(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("records the URL as written, with the name Roam holds", async () => { + mockFetch({ descriptor: descriptorFor({ size: 7 }), bytes: "PNGDATA" }); + + await expect(copy(harness.client)).resolves.toEqual({ + status: "copied", + sourceRef: IMAGE, + contentHash: expect.stringMatching(/^[0-9a-f]{64}$/) as unknown, + sourcePath: "CleanShot 2025-11-16 at 17.14.44@2x.png", + }); + expect([...harness.rows.values()]).toEqual([ + expect.objectContaining({ + filepath: IMAGE, + source_path: "CleanShot 2025-11-16 at 17.14.44@2x.png", + }), + ]); + }); + + it("stores one copy when the same content is referenced twice", async () => { + mockFetch({ descriptor: descriptorFor({ size: 7 }), bytes: "PNGDATA" }); + + await copy(harness.client, IMAGE); + await copy(harness.client, OTHER); + + expect(harness.upload).toHaveBeenCalledTimes(1); + expect(harness.rows.size).toBe(2); + }); + + it("skips an asset above the cap and reports it, without throwing", async () => { + const size = MAX_PUBLISHED_ASSET_BYTES + 1; + mockFetch({ descriptor: descriptorFor({ size }), bytes: "unused" }); + + await expect(copy(harness.client)).resolves.toEqual({ + status: "skipped", + sourceRef: IMAGE, + sourcePath: "CleanShot 2025-11-16 at 17.14.44@2x.png", + reason: "too-large", + size, + limit: MAX_PUBLISHED_ASSET_BYTES, + }); + expect(harness.rows.size).toBe(0); + expect(harness.upload).not.toHaveBeenCalled(); + }); + + it("does not download the bytes of an asset it will skip", async () => { + const { calls } = mockFetch({ + descriptor: descriptorFor({ size: MAX_PUBLISHED_ASSET_BYTES }), + bytes: "unused", + }); + + await copy(harness.client); + + expect(calls).toHaveLength(1); + expect(calls[0]).not.toContain("alt=media"); + }); + + it("skips an over-cap asset whose size the descriptor did not report", async () => { + mockFetch({ + descriptor: descriptorFor({}), + bytes: "x".repeat(MAX_PUBLISHED_ASSET_BYTES + 1), + }); + + const result = await copy(harness.client); + + expect(result.status).toBe("skipped"); + expect(harness.rows.size).toBe(0); + }); + + it("copies an asset whose size the descriptor did not report but is under the cap", async () => { + mockFetch({ descriptor: descriptorFor({}), bytes: "PNGDATA" }); + + const result = await copy(harness.client); + + expect(result.status).toBe("copied"); + expect(harness.rows.size).toBe(1); + }); +}); diff --git a/apps/roam/src/utils/copyAssetToSharedStorage.ts b/apps/roam/src/utils/copyAssetToSharedStorage.ts new file mode 100644 index 000000000..7136feadf --- /dev/null +++ b/apps/roam/src/utils/copyAssetToSharedStorage.ts @@ -0,0 +1,101 @@ +import { addFile } from "@repo/database/lib/files"; +import { + MAX_PUBLISHED_ASSET_BYTES, + isAssetTooLarge, +} from "@repo/database/lib/assetLimits"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import { fetchAssetBytes, fetchAssetDescriptor } from "./fetchRoamAsset"; + +/** + * Copies one Roam asset into shared storage, recording it against the node that + * references it. + * + * The markdown is never rewritten, so the URL that the page already holds is what lands + * in `filepath` and is what a destination matches on. `addFile` hashes the bytes, + * deduplicates through `file_exists`, and resolves a repeated reference, so this does + * none of that itself. + */ + +export type CopiedAsset = { + status: "copied"; + /** The URL as the markdown holds it. */ + sourceRef: string; + /** SHA-256 of the stored bytes, as returned by `addFile`. */ + contentHash: string; + /** The name Roam holds for the asset. */ + sourcePath: string; +}; + +export type SkippedAsset = { + status: "skipped"; + sourceRef: string; + sourcePath: string; + reason: "too-large"; + size: number; + limit: number; +}; + +export type AssetCopyResult = CopiedAsset | SkippedAsset; + +export const copyAssetToSharedStorage = async ({ + client, + spaceId, + sourceLocalId, + assetUrl, + nodeCreated, + nodeLastModified, +}: { + client: DGSupabaseClient; + spaceId: number; + sourceLocalId: string; + assetUrl: string; + /** Used when Roam's storage reports no timestamps of its own. */ + nodeCreated: Date; + nodeLastModified: Date; +}): Promise => { + const descriptor = await fetchAssetDescriptor(assetUrl); + const skip = (size: number): SkippedAsset => ({ + status: "skipped", + sourceRef: assetUrl, + sourcePath: descriptor.filename, + reason: "too-large", + size, + limit: MAX_PUBLISHED_ASSET_BYTES, + }); + + // Decline an oversized asset from the descriptor alone, so its bytes never cross the + // network. Where Roam reports no size, the check moves after the download. + if ( + descriptor.size !== undefined && + isAssetTooLarge({ size: descriptor.size, limit: MAX_PUBLISHED_ASSET_BYTES }) + ) + return skip(descriptor.size); + + const content = await fetchAssetBytes(assetUrl); + if ( + isAssetTooLarge({ + size: content.byteLength, + limit: MAX_PUBLISHED_ASSET_BYTES, + }) + ) + return skip(content.byteLength); + + const contentHash = await addFile({ + client, + spaceId, + sourceLocalId, + fname: assetUrl, + sourcePath: descriptor.filename, + mimetype: descriptor.mimetype, + created: descriptor.createdAt ?? nodeCreated, + lastModified: descriptor.modifiedAt ?? nodeLastModified, + content, + }); + + return { + status: "copied", + sourceRef: assetUrl, + contentHash, + sourcePath: descriptor.filename, + }; +}; diff --git a/packages/database/src/lib/__tests__/files.test.ts b/packages/database/src/lib/__tests__/files.test.ts index 5f6923ea5..32562dd5b 100644 --- a/packages/database/src/lib/__tests__/files.test.ts +++ b/packages/database/src/lib/__tests__/files.test.ts @@ -150,6 +150,15 @@ describe("addFile", () => { ]); }); + it("returns the content hash of the bytes it stored", async () => { + const hash = await publish({ client: harness.client, filename: "d.png" }); + + expect(hash).toMatch(/^[0-9a-f]{64}$/); + expect([...harness.rows.values()]).toEqual([ + expect.objectContaining({ filehash: hash }), + ]); + }); + it("uploads the bytes once when two references share content", async () => { await publish({ client: harness.client, sourcePath: "diagram.png" }); await publish({ diff --git a/packages/database/src/lib/files.ts b/packages/database/src/lib/files.ts index 0933db1d4..fb0a7bd8a 100644 --- a/packages/database/src/lib/files.ts +++ b/packages/database/src/lib/files.ts @@ -24,7 +24,8 @@ export const addFile = async ({ created: Date; lastModified: Date; content: ArrayBuffer; -}): Promise => { + /** Resolves to the content hash, which a publisher needs to describe the stored asset. */ +}): Promise => { // This assumes the content fits in memory. const uint8Array = new Uint8Array(content); const hashBuffer = await crypto.subtle.digest("SHA-256", uint8Array); @@ -75,4 +76,5 @@ export const addFile = async ({ if (updateResult.error) throw updateResult.error; } else throw frefResult.error; } + return hashvalue; }; From f94f28773cb1e5c4f06b336136460119ec275ec0 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Fri, 4 Sep 2026 07:43:07 -0400 Subject: [PATCH 4/5] ENG-1870-Copy-Roam-assets-in-supabase-Task-2.4 --- .../utils/__tests__/publishNodeAssets.test.ts | 460 ++++++++++++++++++ .../__tests__/publishNodesToGroups.test.ts | 53 ++ apps/roam/src/utils/publishNodeAssets.ts | 235 +++++++++ apps/roam/src/utils/publishNodesToGroups.ts | 14 + 4 files changed, 762 insertions(+) create mode 100644 apps/roam/src/utils/__tests__/publishNodeAssets.test.ts create mode 100644 apps/roam/src/utils/publishNodeAssets.ts diff --git a/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts b/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts new file mode 100644 index 000000000..6fc42a4f9 --- /dev/null +++ b/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts @@ -0,0 +1,460 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { CrossAppNode } from "@repo/database/crossAppContracts"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import { contentTypes } from "@repo/content-model"; +import { MAX_PUBLISHED_ASSET_BYTES } from "@repo/database/lib/assetLimits"; +import { publishNodeAssets } from "../publishNodeAssets"; + +const IMAGE = + "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FlqP2ioVNC3.png?alt=media&token=9f1c07a4"; + +type Row = { + space_id?: unknown; + source_local_id?: unknown; + filepath?: unknown; + filehash?: unknown; + source_path?: unknown; +}; + +/** + * A stand-in for Supabase covering what the stage leans on: `my_file_references` answers + * per node, `file_exists` answers from the rows already written, and a delete honours the + * `eq`/`notIn` filters so cleanup can be asserted rather than assumed. + */ +const makeClient = () => { + const rows: Row[] = []; + const upload = vi.fn().mockResolvedValue({ error: null }); + const thenable = (result: unknown) => ({ + then: (resolve: (value: unknown) => unknown) => + Promise.resolve(result).then(resolve), + }); + + type Filter = (row: Row) => boolean; + const filtered = (filters: Filter[]) => + rows.filter((row) => filters.every((f) => f(row))); + + /** Set to make the reference read fail, as an offline client would. */ + let selectError: { message: string } | null = null; + + const selects: number[] = []; + const deletes: number[] = []; + + const selectBuilder = (filters: Filter[]) => { + const builder = { + eq: (column: string, value: unknown) => + selectBuilder([ + ...filters, + (row) => row[column as keyof Row] === value, + ]), + in: (column: string, values: unknown[]) => + selectBuilder([ + ...filters, + (row) => values.includes(row[column as keyof Row]), + ]), + then: (resolve: (value: unknown) => unknown) => { + selects.push(1); + return Promise.resolve( + selectError + ? { data: null, error: selectError } + : { data: filtered(filters), error: null }, + ).then(resolve); + }, + }; + return builder; + }; + + const deleteBuilder = (filters: Filter[]) => ({ + eq: (column: string, value: unknown) => + deleteBuilder([...filters, (row) => row[column as keyof Row] === value]), + notIn: (column: string, values: unknown[]) => + deleteBuilder([ + ...filters, + (row) => !values.includes(row[column as keyof Row]), + ]), + then: (resolve: (value: unknown) => unknown) => { + deletes.push(1); + for (const row of filtered(filters)) rows.splice(rows.indexOf(row), 1); + return Promise.resolve({ error: null }).then(resolve); + }, + }); + + const client = { + rpc: vi.fn((_fn: string, { hashvalue }: { hashvalue: string }) => + Promise.resolve({ + data: rows.some((row) => row.filehash === hashvalue), + error: null, + }), + ), + storage: { from: vi.fn(() => ({ upload })) }, + from: vi.fn(() => ({ + select: vi.fn(() => selectBuilder([])), + delete: vi.fn(() => deleteBuilder([])), + insert: vi.fn((row: Row) => { + rows.push({ ...row }); + return thenable({ error: null }); + }), + update: vi.fn(() => { + const builder = { + eq: vi.fn(() => builder), + then: thenable({ error: null }).then, + }; + return builder; + }), + })), + } as unknown as DGSupabaseClient; + return { + client, + rows, + upload, + filepaths: () => rows.map((r) => r.filepath), + selectCount: () => selects.length, + deleteCount: () => deletes.length, + failReferenceRead: (message: string) => { + selectError = { message }; + }, + }; +}; + +const nodeWith = (markdown: string): CrossAppNode => ({ + localId: "tgWb6JozF", + nodeType: "rCLM0schema", + coreTitle: "Sleep improves memory consolidation", + content: { + direct: { value: "Sleep improves memory consolidation" }, + full: { contentType: contentTypes.markdown, value: markdown }, + }, + createdAt: new Date("2026-06-12T14:00:00.000Z"), + modifiedAt: new Date("2026-06-12T15:00:00.000Z"), + authorId: "maparent", +}); + +const MARKDOWN = `# Sleep improves memory consolidation\n\n![](${IMAGE})\n\n- Supported by [[EVD]] - Rasch & Born 2013\n`; + +const mockFetch = ({ + size = 7, + bytes = "PNGDATA", + descriptorOk = true, +}: { + size?: number; + bytes?: string; + descriptorOk?: boolean; +}) => { + vi.stubGlobal( + "fetch", + vi.fn((input: string) => { + if (!input.includes("alt=media")) + return Promise.resolve({ + ok: descriptorOk, + status: descriptorOk ? 200 : 500, + json: () => + Promise.resolve({ + name: "imgs/app/MAPLab/lqP2ioVNC3.png", + contentType: "image/png", + size: String(size), + metadata: { "file-name": "diagram.png" }, + }), + } as unknown as Response); + return Promise.resolve({ + ok: true, + status: 200, + arrayBuffer: () => + Promise.resolve(new TextEncoder().encode(bytes).buffer), + } as unknown as Response); + }), + ); +}; + +describe("publishNodeAssets", () => { + let harness: ReturnType; + + beforeEach(() => { + harness = makeClient(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("records a reference whose filepath is the URL from the markdown", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + + const results = await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(results).toEqual([ + { + status: "copied", + sourceRef: IMAGE, + sourceLocalId: "tgWb6JozF", + contentHash: expect.stringMatching(/^[0-9a-f]{64}$/) as unknown, + sourcePath: "diagram.png", + }, + ]); + expect(harness.filepaths()).toEqual([IMAGE]); + }); + + it("leaves the published markdown untouched", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(node.content.full?.value).toBe(MARKDOWN); + }); + + it("publishes a node with no assets without recording anything", async () => { + mockFetch({}); + const node = nodeWith("# A title\n\nJust prose.\n"); + + await expect( + publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }), + ).resolves.toEqual([]); + expect(harness.rows).toHaveLength(0); + }); + + it("skips a node with no full content", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + delete node.content.full; + + await expect( + publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }), + ).resolves.toEqual([]); + }); + + it("keeps the rows of a node whose content did not come through", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + // Absent content says nothing about what the node references, so removing its rows + // would destroy data on the strength of a failed fetch. + const withoutContent = nodeWith(MARKDOWN); + delete withoutContent.content.full; + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [withoutContent], + }); + + expect(harness.filepaths()).toEqual([IMAGE]); + expect(harness.deleteCount()).toBe(0); + }); + + it("carries an unfetchable asset out as a failure instead of throwing", async () => { + mockFetch({ descriptorOk: false }); + const node = nodeWith(MARKDOWN); + + const results = await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(results).toEqual([ + { + status: "failed", + sourceRef: IMAGE, + sourceLocalId: "tgWb6JozF", + error: expect.stringContaining( + "Could not read asset descriptor", + ) as unknown, + }, + ]); + expect(node.content.full?.value).toBe(MARKDOWN); + }); + + it("carries an over-cap asset out as a skip", async () => { + mockFetch({ size: MAX_PUBLISHED_ASSET_BYTES + 1 }); + const node = nodeWith(MARKDOWN); + + const results = await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(results[0]).toMatchObject({ + status: "skipped", + reason: "too-large", + sourceLocalId: "tgWb6JozF", + }); + expect(harness.rows).toHaveLength(0); + }); + + it("attributes each asset to the node that references it", async () => { + mockFetch({}); + const first = nodeWith(MARKDOWN); + const second = { ...nodeWith(MARKDOWN), localId: "otherNode" }; + + const results = await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [first, second], + }); + + expect(results.map((r) => r.sourceLocalId)).toEqual([ + "tgWb6JozF", + "otherNode", + ]); + }); + + it("re-publishing an unchanged node fetches nothing and records nothing new", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + const publish = () => + publishNodeAssets({ client: harness.client, spaceId: 20, nodes: [node] }); + + await publish(); + const fetchesAfterFirst = vi.mocked(fetch).mock.calls.length; + + const results = await publish(); + + expect(vi.mocked(fetch).mock.calls).toHaveLength(fetchesAfterFirst); + expect(results).toEqual([ + { + status: "unchanged", + sourceRef: IMAGE, + sourceLocalId: "tgWb6JozF", + contentHash: expect.stringMatching(/^[0-9a-f]{64}$/) as unknown, + sourcePath: "diagram.png", + }, + ]); + expect(harness.filepaths()).toEqual([IMAGE]); + }); + + it("drops the reference to an asset the node no longer embeds", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + expect(harness.filepaths()).toEqual([IMAGE]); + + const withoutImage = nodeWith("# A title\n\nThe image is gone.\n"); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [withoutImage], + }); + + expect(harness.rows).toHaveLength(0); + }); + + it("leaves another node's references alone when cleaning up", async () => { + mockFetch({}); + const other = { ...nodeWith(MARKDOWN), localId: "otherNode" }; + + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [other], + }); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [nodeWith("# A title\n\nNo assets here.\n")], + }); + + expect(harness.rows).toEqual([ + expect.objectContaining({ + source_local_id: "otherNode", + filepath: IMAGE, + }), + ]); + }); + + it("reads every node's references in one query", async () => { + mockFetch({}); + const nodes = ["n1", "n2", "n3"].map((localId) => ({ + ...nodeWith(MARKDOWN), + localId, + })); + + await publishNodeAssets({ client: harness.client, spaceId: 20, nodes }); + + expect(harness.selectCount()).toBe(1); + }); + + it("issues no delete when the node has nothing stale to drop", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + + // A first publish, then an unchanged re-publish: neither has a stale reference. + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(harness.deleteCount()).toBe(0); + }); + + it("replaces the reference when Roam rotates the download token", async () => { + mockFetch({}); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [nodeWith(MARKDOWN)], + }); + + // The token is part of the URL, so a rotation reads as a different reference. + const rotated = `${IMAGE.split("&token=")[0]}&token=rotated`; + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [nodeWith(MARKDOWN.replace(IMAGE, rotated))], + }); + + expect(harness.filepaths()).toEqual([rotated]); + }); + + it("keeps a still-referenced row when the node's references cannot be read", async () => { + mockFetch({}); + const node = nodeWith(MARKDOWN); + await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + harness.failReferenceRead("offline"); + + const results = await publishNodeAssets({ + client: harness.client, + spaceId: 20, + nodes: [node], + }); + + expect(results[0]).toMatchObject({ status: "failed", error: "offline" }); + expect(harness.filepaths()).toEqual([IMAGE]); + }); +}); diff --git a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts index 61658b584..03bad98fa 100644 --- a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts +++ b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts @@ -132,6 +132,9 @@ const makeFakeClient = ({ : [], error: null, }); + // Main's builder already answers what the asset stage asks of `select`: `eq` chains and + // the builder is awaitable on its own, which is the shape `publishNodeAssets` uses when + // it reads a node's existing references with two `eq`s and no `in`. const makeSelectBuilder = (table: string): FakeSelectBuilder => { const builder: FakeSelectBuilder = { url: { search: "" }, @@ -147,9 +150,16 @@ const makeFakeClient = ({ }; return builder; }; + const deleteFilter = (): Record => ({ + eq: () => deleteFilter(), + notIn: () => deleteFilter(), + then: (resolve: (value: unknown) => unknown) => + Promise.resolve({ error: null }).then(resolve), + }); const client = { from: (table: string) => ({ select: () => makeSelectBuilder(table), + delete: () => deleteFilter(), upsert: ( rows: Record[], options: Record, @@ -330,6 +340,49 @@ describe("publishNodesToGroups", () => { }, ); + it("publishes a node whose asset cannot be fetched, with its content intact and the failure reported", async () => { + const assetUrl = + "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FlqP2ioVNC3.png?alt=media&token=9f1c07a4"; + const node = makeCrossAppNode({ uid: "node-1", title: "Claim one" }); + const markdown = `# Claim one\n\n![](${assetUrl})\n`; + node.content.full = { ...node.content.full!, value: markdown }; + vi.stubGlobal( + "fetch", + vi.fn(() => + Promise.resolve({ ok: false, status: 500 } as unknown as Response), + ), + ); + const { client, rpcCalls } = makeFakeClient({}); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [node], + }); + + expect(result.publishedNodeUids).toContain("node-1"); + expect(result.failedUpsertUids).toEqual([]); + expect(result.assetResults).toEqual([ + { + status: "failed", + sourceRef: assetUrl, + sourceLocalId: "node-1", + error: expect.stringContaining( + "Could not read asset descriptor", + ) as unknown, + }, + ]); + // The content that was upserted still carries the asset link. + const upserted = rpcCalls + .flatMap(({ args }) => args.data) + .find((row) => row.source_local_id === "node-1"); + expect(JSON.stringify(upserted)).toContain(assetUrl); + expect(node.content.full?.value).toBe(markdown); + + vi.unstubAllGlobals(); + }); + it("withholds dependent nodes when their schema upsert fails", async () => { const { client, upsertCalls } = makeFakeClient({ rpcResponse: { data: [-1, 2, 3], error: null }, diff --git a/apps/roam/src/utils/publishNodeAssets.ts b/apps/roam/src/utils/publishNodeAssets.ts new file mode 100644 index 000000000..d0b35e4de --- /dev/null +++ b/apps/roam/src/utils/publishNodeAssets.ts @@ -0,0 +1,235 @@ +import type { CrossAppNode } from "@repo/database/crossAppContracts"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import { findAssetReferences } from "./findAssetReferences"; +import { + copyAssetToSharedStorage, + type CopiedAsset, + type SkippedAsset, +} from "./copyAssetToSharedStorage"; + +/** + * The publish stage that copies each node's assets into shared storage. + * + * Runs after the content upsert, not alongside it: `FileReference` has a foreign key to + * `Content`, so the row it hangs off has to exist first. + * + * The node's `full` markdown is read, never written. Publication leaves every asset link + * exactly as the page wrote it, which is what lets the link keep serving as the reference + * a destination matches on, and as the fallback when an asset could not be copied. + * + * Each node's existing references are read once, up front, and drive both halves of + * keeping them in step with the content: a reference already recorded is left alone + * without touching the network, and a recorded reference the content no longer makes is + * deleted. Without the first, a re-publish re-downloads every asset of every node; without + * the second, a destination imports assets the published content stopped mentioning. + */ + +export type FailedAsset = { + status: "failed"; + sourceRef: string; + sourceLocalId: string; + error: string; +}; + +/** A reference already recorded against this node, left as it stands. */ +export type UnchangedAsset = { + status: "unchanged"; + sourceRef: string; + contentHash: string; + sourcePath: string | undefined; +}; + +export type NodeAssetResult = ( + | CopiedAsset + | SkippedAsset + | FailedAsset + | UnchangedAsset +) & { + sourceLocalId: string; +}; + +type ExistingReference = { filehash: string; sourcePath: string | null }; + +/** What is already recorded, per node, keyed by the reference token in `filepath`. */ +type ExistingReferences = Map>; + +/** + * Reads every publishing node's references in one query, rather than one per node: the + * common case is a re-publish where nothing has changed, and that should not cost a round + * trip per node. + */ +const readExistingReferences = async ({ + client, + spaceId, + sourceLocalIds, +}: { + client: DGSupabaseClient; + spaceId: number; + sourceLocalIds: string[]; +}): Promise => { + const { data, error } = await client + .from("my_file_references") + .select("source_local_id, filepath, filehash, source_path") + .eq("space_id", spaceId) + .in("source_local_id", sourceLocalIds); + // A Postgrest error is a plain object, so it has to be wrapped or it reaches the + // result as "[object Object]". + if (error) throw new Error(error.message); + + const byNode: ExistingReferences = new Map(); + for (const row of data) { + // The view's columns are all nullable to the type generator, though the table's are + // not. A row missing any of these cannot be matched against a reference anyway. + if ( + row.source_local_id === null || + row.filepath === null || + row.filehash === null + ) + continue; + const forNode = + byNode.get(row.source_local_id) ?? new Map(); + forNode.set(row.filepath, { + filehash: row.filehash, + sourcePath: row.source_path, + }); + byNode.set(row.source_local_id, forNode); + } + return byNode; +}; + +/** + * Drops references this node no longer makes. + * + * Keyed on what the current markdown says, not on what this run managed to copy: an + * asset that is still referenced but failed to transfer keeps its row, because the + * reference is still true and deleting it would send bytes we still want to the + * collector on a transient network fault. + */ +const removeUnreferencedAssets = async ({ + client, + spaceId, + sourceLocalId, + referenced, +}: { + client: DGSupabaseClient; + spaceId: number; + sourceLocalId: string; + referenced: string[]; +}): Promise => { + let cleanup = client + .from("FileReference") + .delete() + .eq("space_id", spaceId) + .eq("source_local_id", sourceLocalId); + if (referenced.length) cleanup = cleanup.notIn("filepath", referenced); + const { error } = await cleanup; + // Cleanup never fails a publish; the node and its content are already through. + if (error) console.error(error); +}; + +export const publishNodeAssets = async ({ + client, + spaceId, + nodes, +}: { + client: DGSupabaseClient; + spaceId: number; + nodes: CrossAppNode[]; +}): Promise => { + // Nothing is known about a node whose content did not come through, so it is left out + // entirely: nothing copied for it, and nothing removed from it. A node that is here + // with no references is a different case, and still gets its cleanup below. + const nodesWithContent = nodes.flatMap((node) => { + const markdown = node.content.full?.value; + return markdown === undefined + ? [] + : [{ node, referenced: [...new Set(findAssetReferences(markdown))] }]; + }); + if (nodesWithContent.length === 0) return []; + + let existingByNode: ExistingReferences; + try { + existingByNode = await readExistingReferences({ + client, + spaceId, + sourceLocalIds: nodesWithContent.map(({ node }) => node.localId), + }); + } catch (error) { + // Without the existing rows we cannot tell a re-publish from a first one. Copying + // anyway would be correct but wasteful, and removing anything would be unsafe. + const message = error instanceof Error ? error.message : String(error); + return nodesWithContent.flatMap(({ node, referenced }) => + referenced.map( + (sourceRef): NodeAssetResult => ({ + status: "failed", + sourceRef, + sourceLocalId: node.localId, + error: message, + }), + ), + ); + } + + const results: NodeAssetResult[] = []; + for (const { node, referenced } of nodesWithContent) { + const existing = + existingByNode.get(node.localId) ?? new Map(); + + for (const assetUrl of referenced) { + const recorded = existing.get(assetUrl); + if (recorded !== undefined) { + // A Roam storage URL names one immutable upload, because re-uploading a file + // yields a fresh URL. A row under this URL therefore already holds these bytes. + // Nothing to fetch, and no `last_modified` comparison to make. + results.push({ + status: "unchanged", + sourceRef: assetUrl, + contentHash: recorded.filehash, + sourcePath: recorded.sourcePath ?? undefined, + sourceLocalId: node.localId, + }); + continue; + } + // Only this node's rows are consulted, so two nodes embedding the same asset URL + // each download it once. Resolving the second from the row the first wrote is the + // `resolve-repeat-asset-references` proposal, which is post-v0. Do not solve it + // with a byte cache: the fix is to widen this query by `filepath` and to add a + // way to insert a reference row against a hash that is already known. + try { + const result = await copyAssetToSharedStorage({ + client, + spaceId, + sourceLocalId: node.localId, + assetUrl, + nodeCreated: node.createdAt, + nodeLastModified: node.modifiedAt ?? node.createdAt, + }); + results.push({ ...result, sourceLocalId: node.localId }); + } catch (error) { + // One asset never fails its node: the link stays in the published markdown and + // the failure is carried out in the result. + results.push({ + status: "failed", + sourceRef: assetUrl, + sourceLocalId: node.localId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + + // Only when there is actually something to drop. A first publish and an unchanged + // re-publish both have nothing stale, which is the common case, and neither should + // pay for a round trip that would delete nothing. + const hasStaleReference = [...existing.keys()].some( + (filepath) => !referenced.includes(filepath), + ); + if (hasStaleReference) + await removeUnreferencedAssets({ + client, + spaceId, + sourceLocalId: node.localId, + referenced, + }); + } + return results; +}; diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index e357cd777..b41248751 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -27,6 +27,7 @@ import getDiscourseNodes from "./getDiscourseNodes"; import { difference, intersection } from "@repo/utils/setOperations"; import internalError from "./internalError"; import { readImportedSourceIdentity } from "./importedSourceIdentity"; +import { publishNodeAssets, type NodeAssetResult } from "./publishNodeAssets"; export type NodeUidWithType = { uid: string; @@ -222,6 +223,8 @@ type PublishNodesResult = { failedUpsertUids: string[]; okGroupIds: string[]; failedGroupIds: string[]; + /** One entry per asset the published nodes reference. See publishNodeAssets. */ + assetResults: NodeAssetResult[]; }; // Grants a group access to discourse nodes by mirroring the Obsidian @@ -255,6 +258,7 @@ export const publishNodesToGroups = async ({ failedUpsertUids: [], okGroupIds: [], failedGroupIds: [], + assetResults: [], }; if (nodes.length === 0 || groupIds.length === 0) return result; @@ -383,6 +387,16 @@ export const publishNodesToGroups = async ({ nodeUids = [...upsertedNodeUids]; const failedUpsertIds = new Set(result.failedUpsertUids); + // After the content upsert, because FileReference has a foreign key to Content, and + // before the access grants, so a node becomes visible with its assets already recorded. + result.assetResults = await publishNodeAssets({ + client, + spaceId, + nodes: [...nodesByUid.values()].filter((node) => + upsertedNodeUids.has(node.localId), + ), + }); + const resourceAccesses = []; const resourceIds = [...nodeUids, ...nodeSchemaUids]; for (const groupId of groupIds) { From 264ccb5381fd2aa969b6f5e385cc279af4221ab6 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Fri, 4 Sep 2026 10:45:40 -0400 Subject: [PATCH 5/5] ENG-1870-Copy-Roam-assets-in-supabase-Task-2.5 --- apps/roam/src/components/Export.tsx | 27 ++++++- .../utils/__tests__/publishNodeAssets.test.ts | 78 ++++++++++++++++++- apps/roam/src/utils/publishNodeAssets.ts | 56 +++++++++++++ .../database/src/lib/__tests__/files.test.ts | 2 +- 4 files changed, 160 insertions(+), 3 deletions(-) diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index cb0567d71..daa5a1040 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -91,6 +91,7 @@ import { publishNodeUidsWithTypeToGroups, type NodeUidWithType, } from "~/utils/publishNodesToGroups"; +import { summarizeAssetResults } from "~/utils/publishNodeAssets"; import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; import { isNodeSharingEnabled } from "~/components/settings/utils/accessors"; @@ -871,6 +872,7 @@ const ExportDialog: ExportDialogComponent = ({ failedUpsertUids, okGroupIds, failedGroupIds, + assetResults, } = await publishNodeUidsWithTypeToGroups({ client, spaceId: context.spaceId, @@ -881,12 +883,18 @@ const ExportDialog: ExportDialogComponent = ({ const failedNodeCount = failedUpsertUids.filter((uid) => selectedNodeUids.has(uid), ).length; + const assets = summarizeAssetResults(assetResults); posthog.capture("Export Dialog: Publish", { groupCount: okGroupIds.length, publishedNodeCount: publishedNodeUids.length, failedUpsertCount: failedUpsertUids.length, nonDiscourseCount, failedGroupCount: failedGroupIds.length, + assetCopiedCount: assets.copied, + assetUnchangedCount: assets.unchanged, + assetDistinctBlobCount: assets.distinctBlobs, + assetTooLargeCount: assets.tooLarge.length, + assetFailedCount: assets.failed.length, }); const hasPublishedNodes = publishedNodeUids.length > 0; const messages = hasPublishedNodes @@ -908,10 +916,27 @@ const ExportDialog: ExportDialogComponent = ({ failedGroupIds.length === 1 ? "" : "s" } failed.`, ); + // The nodes themselves published either way; their links still point at Roam. + if (assets.tooLarge.length) + messages.push( + `${assets.tooLarge.length} file${ + assets.tooLarge.length === 1 ? " was" : "s were" + } too large to copy.`, + ); + if (assets.failed.length) + messages.push( + `${assets.failed.length} file${ + assets.failed.length === 1 ? "" : "s" + } could not be copied.`, + ); renderToast({ content: messages.join(" "), intent: - failedGroupIds.length || failedNodeCount || !hasPublishedNodes + failedGroupIds.length || + failedNodeCount || + !hasPublishedNodes || + assets.tooLarge.length || + assets.failed.length ? "warning" : "success", id: "query-builder-publish-success", diff --git a/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts b/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts index 6fc42a4f9..aa74e5939 100644 --- a/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts +++ b/apps/roam/src/utils/__tests__/publishNodeAssets.test.ts @@ -3,7 +3,7 @@ import type { CrossAppNode } from "@repo/database/crossAppContracts"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import { contentTypes } from "@repo/content-model"; import { MAX_PUBLISHED_ASSET_BYTES } from "@repo/database/lib/assetLimits"; -import { publishNodeAssets } from "../publishNodeAssets"; +import { publishNodeAssets, summarizeAssetResults } from "../publishNodeAssets"; const IMAGE = "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FlqP2ioVNC3.png?alt=media&token=9f1c07a4"; @@ -458,3 +458,79 @@ describe("publishNodeAssets", () => { expect(harness.filepaths()).toEqual([IMAGE]); }); }); + +describe("summarizeAssetResults", () => { + it("counts copies and keeps skips and failures apart", () => { + const summary = summarizeAssetResults([ + { + status: "copied", + sourceRef: "a", + sourceLocalId: "n", + contentHash: "h", + sourcePath: "a.png", + }, + { + status: "skipped", + sourceRef: "b", + sourceLocalId: "n", + sourcePath: "b.png", + reason: "too-large", + size: 99, + limit: 10, + }, + { + status: "failed", + sourceRef: "c", + sourceLocalId: "n", + error: "boom", + }, + ]); + + expect(summary.copied).toBe(1); + expect(summary.distinctBlobs).toBe(1); + expect(summary.tooLarge.map((a) => a.sourceRef)).toEqual(["b"]); + expect(summary.failed.map((a) => a.sourceRef)).toEqual(["c"]); + }); + + it("reports nothing outstanding when every asset copied", () => { + const summary = summarizeAssetResults([ + { + status: "copied", + sourceRef: "a", + sourceLocalId: "n", + contentHash: "h", + sourcePath: "a.png", + }, + ]); + + expect(summary).toEqual({ + copied: 1, + unchanged: 0, + distinctBlobs: 1, + tooLarge: [], + failed: [], + }); + }); + + it("counts one blob when two nodes reference identical content", () => { + const summary = summarizeAssetResults([ + { + status: "copied", + sourceRef: "a", + sourceLocalId: "n1", + contentHash: "h", + sourcePath: "a.png", + }, + { + status: "copied", + sourceRef: "b", + sourceLocalId: "n2", + contentHash: "h", + sourcePath: "b.png", + }, + ]); + + expect(summary.copied).toBe(2); + expect(summary.distinctBlobs).toBe(1); + }); +}); diff --git a/apps/roam/src/utils/publishNodeAssets.ts b/apps/roam/src/utils/publishNodeAssets.ts index d0b35e4de..a067a4f94 100644 --- a/apps/roam/src/utils/publishNodeAssets.ts +++ b/apps/roam/src/utils/publishNodeAssets.ts @@ -233,3 +233,59 @@ export const publishNodeAssets = async ({ } return results; }; + +export type AssetSummary = { + /** References newly recorded, one per link copied, not one per blob. */ + copied: number; + /** References already recorded, which cost no transfer. */ + unchanged: number; + /** + * Distinct content behind the copied references. Two nodes embedding the same image + * copy twice and store once, so this is what actually landed in the bucket, and it is + * an upper bound on new uploads: a re-publish reuses a blob already there. + */ + distinctBlobs: number; + /** Files declined for being over the cap. Their links stay in the published markdown. */ + tooLarge: SkippedAsset[]; + /** Files that could not be read from Roam's storage. Their links stay too. */ + failed: FailedAsset[]; +}; + +/** One entry per distinct file, so a file embedded in three nodes is reported once. */ +const byDistinctRef = (results: T[]): T[] => { + const seen = new Set(); + return results.filter(({ sourceRef }) => { + if (seen.has(sourceRef)) return false; + seen.add(sourceRef); + return true; + }); +}; + +/** + * Counts what the stage did, for a publish summary. + * + * Skips and failures are kept apart because they mean different things to whoever is + * publishing: an oversized file is a decision they can act on, an unreadable one is a + * fault. Neither stopped the node from publishing. + */ +export const summarizeAssetResults = ( + results: NodeAssetResult[], +): AssetSummary => ({ + copied: results.filter((r) => r.status === "copied").length, + unchanged: results.filter((r) => r.status === "unchanged").length, + distinctBlobs: new Set( + results.flatMap((r) => (r.status === "copied" ? [r.contentHash] : [])), + ).size, + tooLarge: byDistinctRef( + results.filter( + (r): r is SkippedAsset & { sourceLocalId: string } => + r.status === "skipped", + ), + ), + failed: byDistinctRef( + results.filter( + (r): r is FailedAsset & { sourceLocalId: string } => + r.status === "failed", + ), + ), +}); diff --git a/packages/database/src/lib/__tests__/files.test.ts b/packages/database/src/lib/__tests__/files.test.ts index 32562dd5b..01fd006f5 100644 --- a/packages/database/src/lib/__tests__/files.test.ts +++ b/packages/database/src/lib/__tests__/files.test.ts @@ -151,7 +151,7 @@ describe("addFile", () => { }); it("returns the content hash of the bytes it stored", async () => { - const hash = await publish({ client: harness.client, filename: "d.png" }); + const hash = await publish({ client: harness.client, sourcePath: "d.png" }); expect(hash).toMatch(/^[0-9a-f]{64}$/); expect([...harness.rows.values()]).toEqual([