mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-24 19:32:10 +00:00
feat(attachments): typed metadata result and MIME capability helpers (TASK-2420)
`fetchAttachmentMetadata` collapsed 404, every other non-2xx and network throws into a single `null` — and cached it. Two consequences: nothing could treat "gone" as authoritative (so editor undo resurrected deleted attachments as live-looking nodes), and a one-off blip was sticky for the page's lifetime. It now returns a discriminated result: `ok` carries mime/size, `missing` is the authoritative 404, `transient` is everything else. `ok` and `missing` stay cached; `transient` is evicted the moment it settles, so a retry re-issues the HEAD while concurrent callers still share one in-flight request (PLAN-2392 DR-17). Both NodeView consumers act on the split. The chip latches the missing/deleted treatment on `missing` and leaves the filename-guess icon alone on `transient`. The image NodeView probes on load failure — an <img> error event carries no status code, so a deleted row and a network blip are indistinguishable there — and only a 404 latches the permanent placeholder; the toolbar's MIME probe latches too, since it may beat the image to the answer. ItemTimeline drops its probed-mark on `transient` so a blip doesn't permanently strand an entry's metadata. Adds `canOpenInViewer` (DR-16: exact five-type raster allowlist, not an `image/` prefix — SVG carries active content and TIFF/HEIC may not decode) and `canBrowserPreview` (DR-5: that set plus PDF and text/plain) next to `isImage`, which survives unchanged as the general picture predicate. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC
This commit is contained in:
@@ -1,5 +1,11 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { formatBytes, iconForAttachment, isImage } from './display';
|
||||
import {
|
||||
canBrowserPreview,
|
||||
canOpenInViewer,
|
||||
formatBytes,
|
||||
iconForAttachment,
|
||||
isImage
|
||||
} from './display';
|
||||
import { ATTACHMENT_ICON_IDS, ATTACHMENT_ICON_PATHS, iconSvg } from './icons/index';
|
||||
import familyFixture from './mime-families.json';
|
||||
|
||||
@@ -182,4 +188,92 @@ describe('isImage', () => {
|
||||
expect(isImage('image/png')).toBe(true);
|
||||
expect(isImage('application/pdf')).toBe(false);
|
||||
});
|
||||
|
||||
// The DR-16 point in one assertion: isImage is deliberately looser than
|
||||
// the viewer gate, which is why the gate has to be its own helper.
|
||||
it('is looser than the viewer gate — it accepts what canOpenInViewer refuses', () => {
|
||||
expect(isImage('image/svg+xml')).toBe(true);
|
||||
expect(canOpenInViewer('image/svg+xml')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
const VIEWER_TYPES = ['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/avif'];
|
||||
|
||||
describe('canOpenInViewer — PLAN-2392 DR-16', () => {
|
||||
it('accepts exactly the five safe raster types', () => {
|
||||
for (const mime of VIEWER_TYPES) expect(canOpenInViewer(mime)).toBe(true);
|
||||
});
|
||||
|
||||
// The whole reason this isn't `startsWith('image/')`: SVG carries active
|
||||
// content, and TIFF/HEIC are types a browser may simply not decode.
|
||||
it('refuses image/* types outside the allowlist', () => {
|
||||
expect(canOpenInViewer('image/svg+xml')).toBe(false);
|
||||
expect(canOpenInViewer('image/tiff')).toBe(false);
|
||||
expect(canOpenInViewer('image/heic')).toBe(false);
|
||||
expect(canOpenInViewer('image/bmp')).toBe(false);
|
||||
expect(canOpenInViewer('image/jxl')).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses non-image types and a missing MIME', () => {
|
||||
expect(canOpenInViewer('application/pdf')).toBe(false);
|
||||
expect(canOpenInViewer('text/xml')).toBe(false);
|
||||
expect(canOpenInViewer('')).toBe(false);
|
||||
expect(canOpenInViewer(null)).toBe(false);
|
||||
expect(canOpenInViewer(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('normalizes case and parameters before matching', () => {
|
||||
expect(canOpenInViewer('IMAGE/PNG')).toBe(true);
|
||||
expect(canOpenInViewer('image/jpeg; charset=binary')).toBe(true);
|
||||
expect(canOpenInViewer(' image/webp ')).toBe(true);
|
||||
});
|
||||
|
||||
// A prefix test would let `image/svg+xml; charset=utf-8` through some
|
||||
// naive normalizations; pin that it doesn't.
|
||||
it('does not admit a disallowed type by dressing it in parameters', () => {
|
||||
expect(canOpenInViewer('image/svg+xml; charset=utf-8')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('canBrowserPreview — PLAN-2392 DR-5', () => {
|
||||
it('accepts PDF, plain text and the whole viewer raster set', () => {
|
||||
expect(canBrowserPreview('application/pdf')).toBe(true);
|
||||
expect(canBrowserPreview('text/plain')).toBe(true);
|
||||
for (const mime of VIEWER_TYPES) expect(canBrowserPreview(mime)).toBe(true);
|
||||
});
|
||||
|
||||
it('refuses the other text/* subtypes browsers handle inconsistently', () => {
|
||||
expect(canBrowserPreview('text/markdown')).toBe(false);
|
||||
expect(canBrowserPreview('text/csv')).toBe(false);
|
||||
expect(canBrowserPreview('text/xml')).toBe(false);
|
||||
expect(canBrowserPreview('application/xml')).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses office documents, archives and force-downloaded types', () => {
|
||||
expect(canBrowserPreview('application/msword')).toBe(false);
|
||||
expect(
|
||||
canBrowserPreview(
|
||||
'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
|
||||
)
|
||||
).toBe(false);
|
||||
expect(canBrowserPreview('application/zip')).toBe(false);
|
||||
expect(canBrowserPreview('text/html')).toBe(false);
|
||||
expect(canBrowserPreview('application/javascript')).toBe(false);
|
||||
expect(canBrowserPreview('image/svg+xml')).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses a missing MIME and normalizes like the viewer gate', () => {
|
||||
expect(canBrowserPreview(null)).toBe(false);
|
||||
expect(canBrowserPreview(undefined)).toBe(false);
|
||||
expect(canBrowserPreview('')).toBe(false);
|
||||
expect(canBrowserPreview('TEXT/PLAIN; charset=utf-8')).toBe(true);
|
||||
});
|
||||
|
||||
// Superset relationship, stated once so a future edit to either set
|
||||
// can't silently break it.
|
||||
it('is a superset of the viewer gate', () => {
|
||||
for (const mime of VIEWER_TYPES) {
|
||||
expect(canOpenInViewer(mime) && canBrowserPreview(mime)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -221,3 +221,52 @@ export function iconForAttachment(
|
||||
export function isImage(mime: string): boolean {
|
||||
return mime.startsWith('image/');
|
||||
}
|
||||
|
||||
/**
|
||||
* The exact raster types the in-app image viewer may open (PLAN-2392
|
||||
* DR-16). Deliberately an allowlist and NOT an `image/` prefix test:
|
||||
* `image/svg+xml` carries active content, and a legacy row, a
|
||||
* mislabelled upload or an extensionless SVG sniffed as XML can all
|
||||
* arrive wearing an `image/*` label. Formats a browser may not decode
|
||||
* at all (`image/tiff`, `image/heic`) are excluded for the separate
|
||||
* reason that a viewer that silently shows nothing is worse than the
|
||||
* file panel.
|
||||
*
|
||||
* `isImage` survives unchanged as the general "is this a picture"
|
||||
* predicate (icon choice, grouping); this is the narrower question of
|
||||
* what may be handed to the viewer.
|
||||
*/
|
||||
const VIEWER_MIMES: ReadonlySet<string> = new Set([
|
||||
'image/png',
|
||||
'image/jpeg',
|
||||
'image/gif',
|
||||
'image/webp',
|
||||
'image/avif'
|
||||
]);
|
||||
|
||||
/**
|
||||
* Additional types a browser renders honestly in a new tab (PLAN-2392
|
||||
* DR-5). PDF and plain text only — every other `text/*` subtype
|
||||
* (markdown, CSV, XML) is downloaded or rendered inconsistently across
|
||||
* browsers, and office documents, archives and the types the server
|
||||
* force-downloads (HTML, JS) never preview. Those surfaces offer
|
||||
* Download alone.
|
||||
*/
|
||||
const BROWSER_PREVIEW_MIMES: ReadonlySet<string> = new Set([
|
||||
'application/pdf',
|
||||
'text/plain'
|
||||
]);
|
||||
|
||||
/** May this MIME be opened in the in-app image viewer? (DR-16) */
|
||||
export function canOpenInViewer(mime: string | null | undefined): boolean {
|
||||
return VIEWER_MIMES.has(normalizeMime(mime));
|
||||
}
|
||||
|
||||
/**
|
||||
* May this MIME be handed to the browser to display — the viewer's
|
||||
* raster set plus PDF and plain text? (DR-5)
|
||||
*/
|
||||
export function canBrowserPreview(mime: string | null | undefined): boolean {
|
||||
const m = normalizeMime(mime);
|
||||
return VIEWER_MIMES.has(m) || BROWSER_PREVIEW_MIMES.has(m);
|
||||
}
|
||||
|
||||
@@ -306,19 +306,30 @@ export const AttachmentChip = Node.create<AttachmentChipOptions>({
|
||||
this.options.workspaceSlug,
|
||||
forUuid,
|
||||
this.options.getDownloadUrl,
|
||||
).then((meta) => {
|
||||
if (!meta) return;
|
||||
).then((result) => {
|
||||
if (destroyed) return; // NodeView torn down while HEAD was in flight
|
||||
if (deleted) return; // the target is gone; don't un-mark the chip
|
||||
if (currentUuid !== forUuid) return; // superseded
|
||||
currentMime = meta.mime;
|
||||
// A transient failure says nothing about whether the row
|
||||
// exists — keep the filename-guess icon and stay
|
||||
// retryable (PLAN-2392 DR-17).
|
||||
if (result.status === 'transient') return;
|
||||
// A 404 IS authoritative. This is the path editor undo
|
||||
// takes: undo restores the chip node, but the delete was a
|
||||
// REST row mutation Tiptap's history can't roll back, so
|
||||
// the chip must render dead rather than link to a 404.
|
||||
if (result.status === 'missing') {
|
||||
markDeleted();
|
||||
return;
|
||||
}
|
||||
currentMime = result.mime;
|
||||
refreshIcon();
|
||||
// The shared formatter renders "0 B" and doesn't guard
|
||||
// non-finite input; a chip with no known size should show
|
||||
// nothing at all, so the conditional lives here rather
|
||||
// than in the helper (PLAN-2392 DR-3b).
|
||||
const size =
|
||||
Number.isFinite(meta.size) && meta.size > 0 ? formatBytes(meta.size) : '';
|
||||
Number.isFinite(result.size) && result.size > 0 ? formatBytes(result.size) : '';
|
||||
sizeEl.textContent = size ? `· ${size}` : '';
|
||||
});
|
||||
};
|
||||
|
||||
@@ -310,9 +310,50 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
// queued, so a superseded load simply has no callback left to run.
|
||||
let detachLoadListeners = () => {};
|
||||
|
||||
/**
|
||||
* Latch the permanent placeholder for a row the server says is
|
||||
* gone. Same end state as the deletion broadcast, reached by a
|
||||
* different route: the broadcast only fires for a delete that
|
||||
* happened in THIS tab's session, while this covers a node whose
|
||||
* row was already gone when it rendered — which is exactly what
|
||||
* editor undo produces (PLAN-2392 DR-17). Tiptap/Yjs history
|
||||
* owns the document; the delete was a REST row mutation it can't
|
||||
* roll back, so undo restores a node pointing at nothing.
|
||||
*/
|
||||
function latchMissing(forUuid: string) {
|
||||
if (deleted) return;
|
||||
if (!forUuid || currentUuid !== forUuid) return;
|
||||
deleted = true;
|
||||
// Same reason the deletion listener does this: an in-flight
|
||||
// load's `load` event would otherwise paint the image back.
|
||||
detachLoadListeners();
|
||||
showMissing();
|
||||
}
|
||||
|
||||
/**
|
||||
* An <img> `error` event carries no status code, so a deleted row
|
||||
* and a network blip are indistinguishable at that layer — which
|
||||
* is why every load failure has to stay retryable by default. A
|
||||
* HEAD probe is what tells them apart: only a 404 latches, and a
|
||||
* `transient` result leaves the retryable placeholder exactly as
|
||||
* it was (and is not cached, so Retry re-issues the HEAD).
|
||||
*/
|
||||
function probeForMissing(forUuid: string) {
|
||||
if (!forUuid || !opts.workspaceSlug || deleted) return;
|
||||
void fetchAttachmentMetadata(opts.workspaceSlug, forUuid, opts.getDownloadUrl).then(
|
||||
(result) => {
|
||||
if (result.status === 'missing') latchMissing(forUuid);
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
function loadImage(url: string) {
|
||||
detachLoadListeners();
|
||||
const onError = () => showMissing();
|
||||
const forUuid = currentUuid;
|
||||
const onError = () => {
|
||||
showMissing();
|
||||
probeForMissing(forUuid);
|
||||
};
|
||||
const onLoad = () => resetMissing();
|
||||
img.addEventListener('error', onError, { once: true });
|
||||
img.addEventListener('load', onLoad, { once: true });
|
||||
@@ -404,12 +445,19 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
if (currentUuid && opts.workspaceSlug) {
|
||||
const probeUuid = currentUuid;
|
||||
fetchAttachmentMetadata(opts.workspaceSlug, probeUuid, opts.getDownloadUrl).then(
|
||||
(meta) => {
|
||||
(result) => {
|
||||
// Bail if the NodeView's uuid changed (rotate/peer
|
||||
// op) while the probe was in flight — otherwise
|
||||
// we'd cache stale MIME state for the new image.
|
||||
if (!toolbar || currentUuid !== probeUuid) return;
|
||||
toolbarMime = meta?.mime ?? null;
|
||||
if (currentUuid !== probeUuid) return;
|
||||
// A 404 here is the same authoritative signal the
|
||||
// load path acts on (DR-17), and this probe may
|
||||
// well beat the <img> to it.
|
||||
if (result.status === 'missing') latchMissing(probeUuid);
|
||||
if (!toolbar) return;
|
||||
// `transient` leaves the MIME unknown rather than
|
||||
// wrong: gating falls back to supportedFormats.
|
||||
toolbarMime = result.status === 'ok' ? result.mime : null;
|
||||
refresh();
|
||||
}
|
||||
);
|
||||
@@ -591,9 +639,11 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
opts.workspaceSlug,
|
||||
probeUuid,
|
||||
opts.getDownloadUrl
|
||||
).then((meta) => {
|
||||
if (!toolbar || currentUuid !== probeUuid) return;
|
||||
toolbarMime = meta?.mime ?? null;
|
||||
).then((result) => {
|
||||
if (currentUuid !== probeUuid) return;
|
||||
if (result.status === 'missing') latchMissing(probeUuid);
|
||||
if (!toolbar) return;
|
||||
toolbarMime = result.status === 'ok' ? result.mime : null;
|
||||
refresh();
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,226 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import {
|
||||
fetchAttachmentMetadata,
|
||||
invalidateAttachmentMetadata,
|
||||
mimeToFormat
|
||||
} from './attachment-metadata';
|
||||
|
||||
// PLAN-2392 DR-17. The three arms exist so a caller can tell "the row is
|
||||
// gone" (latch the placeholder — editor undo must not resurrect a deleted
|
||||
// attachment) apart from "the request didn't make it" (stay retryable).
|
||||
// The old helper collapsed both into `null` AND cached it, which made a
|
||||
// one-off blip permanently sticky for the page's lifetime.
|
||||
|
||||
const url = (id: string) => `/api/v1/workspaces/ws/attachments/${id}`;
|
||||
|
||||
/** A HEAD response with the headers the helper reads. */
|
||||
function head(status: number, headers: Record<string, string> = {}): Response {
|
||||
return new Response(null, { status, headers });
|
||||
}
|
||||
|
||||
let fetchMock: ReturnType<typeof vi.fn>;
|
||||
|
||||
// Each test uses a fresh uuid AND invalidates it, because the module-level
|
||||
// promise cache is process-wide and deliberately outlives a single probe.
|
||||
let counter = 0;
|
||||
function freshUuid(): string {
|
||||
counter += 1;
|
||||
return `uuid-${counter}`;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
describe('fetchAttachmentMetadata — result arms', () => {
|
||||
it('returns ok with the parsed MIME and size on 200', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(
|
||||
head(200, { 'content-type': 'image/png; charset=binary', 'content-length': '4096' })
|
||||
);
|
||||
|
||||
const result = await fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
expect(result).toEqual({ status: 'ok', mime: 'image/png', size: 4096 });
|
||||
// HEAD, not GET — a GET would pull the whole blob across the wire.
|
||||
expect(fetchMock).toHaveBeenCalledWith(url(uuid), {
|
||||
method: 'HEAD',
|
||||
credentials: 'same-origin'
|
||||
});
|
||||
});
|
||||
|
||||
it('falls back to a zero size when content-length is absent or junk', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(200, { 'content-type': 'application/pdf' }));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({
|
||||
status: 'ok',
|
||||
mime: 'application/pdf',
|
||||
size: 0
|
||||
});
|
||||
});
|
||||
|
||||
it('reports 404 as missing — the authoritative "row is gone" answer', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(404));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' });
|
||||
});
|
||||
|
||||
it('reports a 500 as transient, not missing', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(500));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' });
|
||||
});
|
||||
|
||||
it('reports a mid-session 403 as transient, not missing', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(403));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' });
|
||||
});
|
||||
|
||||
it('reports a network throw as transient rather than rejecting', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockRejectedValue(new TypeError('Failed to fetch'));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('fetchAttachmentMetadata — caching is per-arm', () => {
|
||||
it('caches an ok result for the page lifetime', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/webp', 'content-length': '10' }));
|
||||
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('caches a missing result — deletion is durable, so stop asking', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(404));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' });
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' });
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does NOT cache a transient failure — a retry re-issues the HEAD', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValueOnce(head(503));
|
||||
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' });
|
||||
|
||||
// ...and the row was fine all along: the second probe must reach the
|
||||
// network and see that, rather than replaying the cached failure.
|
||||
fetchMock.mockResolvedValueOnce(
|
||||
head(200, { 'content-type': 'image/jpeg', 'content-length': '7' })
|
||||
);
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({
|
||||
status: 'ok',
|
||||
mime: 'image/jpeg',
|
||||
size: 7
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('does NOT cache a network throw either', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockRejectedValueOnce(new TypeError('offline'));
|
||||
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
fetchMock.mockResolvedValueOnce(head(404));
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' });
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('still shares one in-flight HEAD between concurrent callers that fail', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(500));
|
||||
|
||||
const [a, b] = await Promise.all([
|
||||
fetchAttachmentMetadata('ws', uuid, url),
|
||||
fetchAttachmentMetadata('ws', uuid, url)
|
||||
]);
|
||||
|
||||
expect(a).toEqual({ status: 'transient' });
|
||||
expect(b).toEqual({ status: 'transient' });
|
||||
// Eviction happens when the promise SETTLES, so dedupe survives.
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keys the cache by workspace as well as uuid', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/gif', 'content-length': '1' }));
|
||||
|
||||
await fetchAttachmentMetadata('ws-a', uuid, url);
|
||||
await fetchAttachmentMetadata('ws-b', uuid, url);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('invalidate drops a cached entry so the next call refetches', async () => {
|
||||
const uuid = freshUuid();
|
||||
fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/png', 'content-length': '2' }));
|
||||
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
invalidateAttachmentMetadata('ws', uuid);
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('transient eviction cannot delete a newer entry installed by an invalidate race', async () => {
|
||||
const uuid = freshUuid();
|
||||
let releaseFirst!: (r: Response) => void;
|
||||
fetchMock.mockImplementationOnce(
|
||||
() => new Promise<Response>((resolve) => (releaseFirst = resolve))
|
||||
);
|
||||
|
||||
const slow = fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
// A delete/transform lands, the entry is invalidated, and a fresh
|
||||
// probe caches a good result — all before the first HEAD settles.
|
||||
invalidateAttachmentMetadata('ws', uuid);
|
||||
fetchMock.mockResolvedValueOnce(
|
||||
head(200, { 'content-type': 'image/avif', 'content-length': '3' })
|
||||
);
|
||||
await fetchAttachmentMetadata('ws', uuid, url);
|
||||
|
||||
releaseFirst(head(500));
|
||||
expect(await slow).toEqual({ status: 'transient' });
|
||||
|
||||
// The newer, good entry survived the older promise's eviction.
|
||||
expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({
|
||||
status: 'ok',
|
||||
mime: 'image/avif',
|
||||
size: 3
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('mimeToFormat', () => {
|
||||
it('maps the recognized image MIMEs to the server-side format names', () => {
|
||||
expect(mimeToFormat('image/jpeg')).toBe('jpeg');
|
||||
expect(mimeToFormat('image/jpg')).toBe('jpeg');
|
||||
expect(mimeToFormat('image/heif')).toBe('heic');
|
||||
expect(mimeToFormat('IMAGE/PNG')).toBe('png');
|
||||
});
|
||||
|
||||
it('returns null for non-images and unknown image subtypes', () => {
|
||||
expect(mimeToFormat('application/pdf')).toBeNull();
|
||||
expect(mimeToFormat('image/jxl')).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -14,9 +14,9 @@
|
||||
* immutable (the row is content-addressed; transforms produce
|
||||
* NEW rows), so there's no staleness concern.
|
||||
*
|
||||
* Skipped silently when no workspace context is available (e.g.
|
||||
* headless rendering / SSR) — callers see `null` and degrade UI
|
||||
* accordingly without raising errors.
|
||||
* Callers never see an exception: every failure is reported through the
|
||||
* discriminated result below, so a surface with no workspace context
|
||||
* (headless rendering / SSR) simply doesn't call this at all.
|
||||
*/
|
||||
|
||||
/** Variants the download URL builder must support. Mirrors AttachmentImage. */
|
||||
@@ -30,38 +30,83 @@ export interface AttachmentMetadata {
|
||||
size: number;
|
||||
}
|
||||
|
||||
const cache = new Map<string, Promise<AttachmentMetadata | null>>();
|
||||
/**
|
||||
* The outcome of a metadata probe (PLAN-2392 DR-17).
|
||||
*
|
||||
* The three arms exist because callers need to tell "the row is gone"
|
||||
* apart from "the request didn't make it", and the old `null` return
|
||||
* collapsed both:
|
||||
*
|
||||
* - `ok` — the HEAD succeeded; `mime` / `size` are usable.
|
||||
* - `missing` — the server answered 404. AUTHORITATIVE: the row is
|
||||
* gone, and a caller may latch a permanent
|
||||
* missing-attachment placeholder on it. This is what
|
||||
* keeps editor undo from resurrecting a deleted
|
||||
* attachment as a live-looking node.
|
||||
* - `transient` — any other non-2xx (5xx, 401/403 mid-session, a
|
||||
* proxy hiccup) or a network throw. Says NOTHING
|
||||
* about whether the row exists; callers keep whatever
|
||||
* they were showing and stay retryable.
|
||||
*/
|
||||
export type AttachmentMetadataResult =
|
||||
| ({ status: 'ok' } & AttachmentMetadata)
|
||||
| { status: 'missing' }
|
||||
| { status: 'transient' };
|
||||
|
||||
const cache = new Map<string, Promise<AttachmentMetadataResult>>();
|
||||
|
||||
/**
|
||||
* Fetch (or read from cache) the MIME + size for an attachment. The
|
||||
* server registers HEAD alongside GET (TASK-877); chi doesn't auto-
|
||||
* route HEAD on GET handlers, so this must use HEAD — a GET would
|
||||
* pull the entire blob across the wire.
|
||||
*
|
||||
* Caching is per-arm (PLAN-2392 DR-17). `ok` and `missing` are both
|
||||
* durable facts about a content-addressed row, so they're kept for the
|
||||
* page lifetime. A `transient` result is NOT — it's evicted the moment
|
||||
* it settles, so a blip can't make a live attachment look permanently
|
||||
* unreadable for the rest of the session. The entry is still installed
|
||||
* BEFORE the request settles, so concurrent callers for the same key
|
||||
* share one in-flight HEAD either way; only the settled failure is
|
||||
* dropped.
|
||||
*/
|
||||
export function fetchAttachmentMetadata(
|
||||
workspaceSlug: string,
|
||||
uuid: string,
|
||||
getDownloadUrl: AttachmentUrlBuilder
|
||||
): Promise<AttachmentMetadata | null> {
|
||||
): Promise<AttachmentMetadataResult> {
|
||||
const key = `${workspaceSlug}:${uuid}`;
|
||||
const existing = cache.get(key);
|
||||
if (existing) return existing;
|
||||
const promise: Promise<AttachmentMetadata | null> = (async () => {
|
||||
const promise: Promise<AttachmentMetadataResult> = (async () => {
|
||||
try {
|
||||
const resp = await fetch(getDownloadUrl(uuid), {
|
||||
method: 'HEAD',
|
||||
credentials: 'same-origin'
|
||||
});
|
||||
if (!resp.ok) return null;
|
||||
if (resp.status === 404) return { status: 'missing' as const };
|
||||
if (!resp.ok) return { status: 'transient' as const };
|
||||
const ctype = resp.headers.get('content-type') ?? '';
|
||||
const mime = ctype.split(';')[0].trim();
|
||||
const len = parseInt(resp.headers.get('content-length') ?? '0', 10);
|
||||
return { mime, size: Number.isFinite(len) && len >= 0 ? len : 0 };
|
||||
return {
|
||||
status: 'ok' as const,
|
||||
mime,
|
||||
size: Number.isFinite(len) && len >= 0 ? len : 0
|
||||
};
|
||||
} catch {
|
||||
return null;
|
||||
return { status: 'transient' as const };
|
||||
}
|
||||
})();
|
||||
cache.set(key, promise);
|
||||
// Evict a transient failure once it settles. The identity check keeps
|
||||
// this from deleting a NEWER entry installed by an invalidate-then-
|
||||
// refetch that raced this promise's resolution.
|
||||
void promise.then((result) => {
|
||||
if (result.status === 'transient' && cache.get(key) === promise) {
|
||||
cache.delete(key);
|
||||
}
|
||||
});
|
||||
return promise;
|
||||
}
|
||||
|
||||
|
||||
@@ -120,7 +120,18 @@
|
||||
fetchAttachmentMetadata(wsSlug, uuid, (id, variant) =>
|
||||
attachmentDownloadUrl(wsSlug, id, variant)
|
||||
).then((m) => {
|
||||
if (!m) return;
|
||||
// A transient failure (5xx / network) is not evidence about the
|
||||
// row, and the helper deliberately doesn't cache it — so drop the
|
||||
// probed mark too, or this panel would never ask again for the
|
||||
// rest of the session (PLAN-2392 DR-17). A `missing` result IS
|
||||
// authoritative: leave the mark set and leave `attMeta` without an
|
||||
// entry, which is what the renderer already degrades to a missing
|
||||
// placeholder on.
|
||||
if (m.status === 'transient') {
|
||||
probed.delete(uuid);
|
||||
return;
|
||||
}
|
||||
if (m.status !== 'ok') return;
|
||||
if (reqWs !== wsSlug) return;
|
||||
const next = new Map(attMeta);
|
||||
// filename is left empty — the markdown alt text is the chip/img
|
||||
|
||||
Reference in New Issue
Block a user