Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/studio-media-speed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@cloudhal/api": patch
"@cloudhal/cli": patch
---

Studio's images load faster and stop re-downloading. Preview URLs carry the file's hash (`?v=`), so the browser keeps them for a day; without it the content route answers `private, no-cache` and a 304 through the ETag, instead of `no-store`. While an image loads, its tile shows the image's own colour and small blurred preview (computed at upload) instead of a spinner.
1 change: 1 addition & 0 deletions apps/studio/src/components/Media.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export function AssetFrame({
fit={size === "card" ? "contain" : "cover"}
src={src}
alt={alt ?? ""}
{...(asset.image ? { placeholder: asset.image } : {})}
labels={{
empty: t("media.preview.empty"),
loading: t("media.preview.loading"),
Expand Down
8 changes: 6 additions & 2 deletions apps/studio/src/lib/api/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ export const mediaApi = {
method: "POST",
...json({ files }),
}),
/** Studio's own, session-authenticated copy of the bytes (drafts included, never cached). */
contentUrl: (siteId: string, id: string) => `${assetPath(siteId, id)}/content`,
/**
* Studio's own, session-authenticated copy of the bytes (drafts included). With the file's hash
* (`v`, its first 16 hex digits) the browser keeps it for a day instead of asking again.
*/
contentUrl: (siteId: string, id: string, sha256?: string) =>
`${assetPath(siteId, id)}/content${sha256 ? `?v=${sha256.slice(0, 16)}` : ""}`,
};
2 changes: 1 addition & 1 deletion apps/studio/src/lib/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export function assetPreviewUrl(siteId: string, asset: Asset): string | undefine
if (asset.kind !== "image") return undefined;
if (asset.storage.kind === "external")
return /^https:\/\//.test(asset.storage.url) ? asset.storage.url : undefined;
return api.assets.contentUrl(siteId, asset.id);
return api.assets.contentUrl(siteId, asset.id, asset.sha256);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion apps/studio/src/routes/AssetPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export function AssetPage() {
const readOnlyReason = data.editable ? undefined : (upload.reason ?? t("media.detail.notOwn"));
const show = (e: unknown) => showErrors && e !== undefined;
const usedCount = new Set(data.uses.map((u) => u.nodeId)).size;
const contentUrl = api.assets.contentUrl(siteId, asset.id);
const contentUrl = api.assets.contentUrl(siteId, asset.id, asset.sha256);
const ext = asset.slug ? data.url.split(".").pop() : "";
const prefix = asset.sha256.slice(0, 16);

Expand Down
2 changes: 1 addition & 1 deletion apps/studio/test/design-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ describe("design rules (DESIGN.md)", () => {
}
// The content route is the only asset source, and it is Studio's own (same origin).
const media = readFileSync(path.join(SRC, "lib/media.ts"), "utf8");
expect(media).toMatch(/return api\.assets\.contentUrl\(siteId, asset\.id\)/);
expect(media).toMatch(/return api\.assets\.contentUrl\(siteId, asset\.id, asset\.sha256\)/);
});

it("file bytes go up only through a single-use ticket (§2u, RFC-0004 §5.5)", () => {
Expand Down
3 changes: 2 additions & 1 deletion apps/studio/test/media-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ describe("showing files (§2u)", () => {
});

it("previews stored images from Studio's private content route, never the public URL", () => {
// `v` names the bytes, so the browser may keep them (a day)
expect(assetPreviewUrl("site_1", asset())).toBe(
"/api/sites/site_1/assets/asset_photo01/content",
`/api/sites/site_1/assets/asset_photo01/content?v=${"ab".repeat(8)}`,
);
expect(
assetPreviewUrl("site_1", asset({ kind: "data", mediaType: "text/csv", image: undefined })),
Expand Down
33 changes: 33 additions & 0 deletions packages/api/src/routes/asset-content-headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Headers of Studio's private copy of an asset's bytes (`GET …/assets/:id/content`).
*/
import type { Asset } from "@cloudhal/core-contracts";
import { MEDIA_TYPES, mediaExtension, servedContentType } from "@cloudhal/core-ts/media";

/**
* Response headers for Studio's private previews (never shared caches, never executed). A URL
* naming the bytes (`?v=` = the first 16 hex digits of their SHA-256) cannot change, so the
* browser keeps it for a day: Studio's media grid no longer downloads every image on every
* visit. A day, not a year, so a file that is taken down does not linger in browsers. Without
* `v`, the browser asks again each time and gets a 304 through the ETag.
*/
export function contentHeaders(asset: Asset, version: string | undefined): Record<string, string> {
const info = MEDIA_TYPES[asset.mediaType];
const svg = asset.mediaType === "image/svg+xml";
const filename = `${asset.slug}.${mediaExtension(asset.mediaType)}`;
return {
"content-type": servedContentType(asset.mediaType),
"content-disposition":
info?.disposition === "inline" ? "inline" : `attachment; filename="${filename}"`,
"cache-control":
version && asset.sha256.startsWith(version) ? "private, max-age=86400" : "private, no-cache",
"x-content-type-options": "nosniff",
"cross-origin-resource-policy": "same-origin",
"content-security-policy": svg
? "default-src 'none'; img-src data:; style-src 'unsafe-inline'; sandbox"
: asset.mediaType === "application/pdf"
? "frame-ancestors 'self'"
: "default-src 'none'; frame-ancestors 'self'",
etag: `"${asset.sha256}"`,
};
}
26 changes: 3 additions & 23 deletions packages/api/src/routes/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*
* Every change writes one audit row (`asset.*`, §6.3); REST writes by people take no snapshot.
*/

import { holds } from "@cloudhal/agent-runtime";
import type { Actor, Asset, Site } from "@cloudhal/core-contracts";
import { InvariantError, termName, type UploadTicket } from "@cloudhal/core-ts";
Expand All @@ -22,7 +23,6 @@ import {
mediaPath,
mediaTypeFromFilename,
normalizeMediaType,
servedContentType,
} from "@cloudhal/core-ts/media";
import { BlobIntegrityError, readAllBytes, sha256OfBytes } from "@cloudhal/storage";
import { type Context, Hono } from "hono";
Expand All @@ -32,6 +32,7 @@ import { actorOf, requireSite } from "../governance.ts";
import { personOf } from "../own.ts";
import { param } from "../params.ts";
import { signUploadTicket, verifyUploadTicket } from "../uploads.ts";
import { contentHeaders } from "./asset-content-headers.ts";

/** RFC-0004 §5.1: per request, per actor and per ticket. */
export const UPLOAD_LIMITS = {
Expand Down Expand Up @@ -417,27 +418,6 @@ function uploadError(e: unknown, ticket: UploadTicket): never {
throw e;
}

/** Response headers for Studio's private previews (never cached, never executed). */
function contentHeaders(asset: Asset): Record<string, string> {
const info = MEDIA_TYPES[asset.mediaType];
const svg = asset.mediaType === "image/svg+xml";
const filename = `${asset.slug}.${mediaExtension(asset.mediaType)}`;
return {
"content-type": servedContentType(asset.mediaType),
"content-disposition":
info?.disposition === "inline" ? "inline" : `attachment; filename="${filename}"`,
"cache-control": "private, no-store",
"x-content-type-options": "nosniff",
"cross-origin-resource-policy": "same-origin",
"content-security-policy": svg
? "default-src 'none'; img-src data:; style-src 'unsafe-inline'; sandbox"
: asset.mediaType === "application/pdf"
? "frame-ancestors 'self'"
: "default-src 'none'; frame-ancestors 'self'",
etag: `"${asset.sha256}"`,
};
}

export const assets: Hono<Env> = new Hono<Env>()
.get("/", async (c) => {
const store = c.get("store");
Expand Down Expand Up @@ -657,7 +637,7 @@ export const assets: Hono<Env> = new Hono<Env>()
const state = await store.blobState(asset.siteId, asset.sha256);
if (state?.state === "purged")
throw apiError(410, "purged", "this file was taken down; its bytes are gone");
const headers = contentHeaders(asset);
const headers = contentHeaders(asset, c.req.query("v") || undefined);
if (c.req.header("if-none-match") === headers.etag) return c.body(null, 304, headers);
const key = `media/${asset.siteId}/${asset.sha256}`;
const blobs = store.blobs;
Expand Down
17 changes: 16 additions & 1 deletion packages/api/src/testing-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,22 @@ export function describeAssetsApi(label: string, backend: AssetsApiBackend): voi
headers: { "x-test-user": "1" },
});
expect(content.status).toBe(200);
expect(content.headers.get("cache-control")).toBe("private, no-store");
// Without the hash the browser asks again, and the ETag answers 304; with it, it keeps
// the bytes for a day (the URL cannot change meaning).
expect(content.headers.get("cache-control")).toBe("private, no-cache");
const again = await app.request(`${base()}/${asset.id}/content`, {
headers: { "x-test-user": "1", "if-none-match": `"${asset.sha256}"` },
});
expect(again.status).toBe(304);
const pinned = await app.request(
`${base()}/${asset.id}/content?v=${asset.sha256.slice(0, 16)}`,
{ headers: { "x-test-user": "1" } },
);
expect(pinned.headers.get("cache-control")).toBe("private, max-age=86400");
const stale = await app.request(`${base()}/${asset.id}/content?v=0000000000000000`, {
headers: { "x-test-user": "1" },
});
expect(stale.headers.get("cache-control")).toBe("private, no-cache");
expect(content.headers.get("x-content-type-options")).toBe("nosniff");
expect(content.headers.get("content-type")).toBe("image/jpeg");
const stored = new Uint8Array(await content.arrayBuffer());
Expand Down
24 changes: 22 additions & 2 deletions packages/ui/src/blocks/SeoPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export interface ImagePreviewProps {
caption?: ReactNode;
/** Replaces the caption while the image is broken, e.g. why a site path cannot load here. */
errorHint?: ReactNode;
/**
* Shown while the image loads, in place of the spinner: its dominant colour and its small
* blurred preview (`Asset.image.color`, `.lqip`, computed at upload, RFC-0028).
*/
placeholder?: { color?: string; lqip?: string };
/** Called once per source with the image's natural size. */
onLoad?: (size: { width: number; height: number }) => void;
onStatusChange?: (status: ImagePreviewStatus) => void;
Expand All @@ -54,6 +59,7 @@ export function ImagePreview({
onLoad,
onStatusChange,
className,
placeholder,
}: ImagePreviewProps) {
const [state, setState] = useState<{ src: string | undefined; status: ImagePreviewStatus }>({
src,
Expand All @@ -74,6 +80,17 @@ export function ImagePreview({
const compact = thumb || tile;
const iconSize = thumb ? 16 : tile ? 24 : 32;
const note = status === "error" ? errorHint : caption;
// The image's own colour and preview stand in while it loads (data, not a design colour).
const painted = status === "loading" && (placeholder?.color || placeholder?.lqip);
const paint = painted
? {
backgroundColor: placeholder?.color,
backgroundImage: placeholder?.lqip ? `url("${placeholder.lqip}")` : undefined,
backgroundSize: fit === "contain" ? "contain" : "cover",
backgroundPosition: "center",
backgroundRepeat: "no-repeat",
}
: undefined;
const frame = (
<div
data-image-status={status}
Expand All @@ -86,6 +103,7 @@ export function ImagePreview({
: "aspect-[1.91/1] w-full rounded-lg",
className,
)}
style={paint}
>
{src && status !== "error" && (
<img
Expand Down Expand Up @@ -119,13 +137,15 @@ export function ImagePreview({
title={compact ? labelFor(status, labels) : undefined}
>
{status === "loading" ? (
<Loader size={compact ? "sm" : "base"} />
painted ? null : (
<Loader size={compact ? "sm" : "base"} />
)
) : status === "error" ? (
<ImageBroken size={iconSize} aria-hidden />
) : (
<ImageIcon size={iconSize} aria-hidden />
)}
{compact ? (
{compact || painted ? (
<span className="sr-only">{labelFor(status, labels)}</span>
) : (
<Text as="span" variant="secondary" size="sm">
Expand Down
30 changes: 30 additions & 0 deletions packages/ui/test/image-preview-placeholder.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { render } from "@testing-library/react";
import { describe, expect, it } from "vitest";
import { ImagePreview } from "../src/blocks/SeoPreview.tsx";

const labels = { empty: "No image", loading: "Loading image", error: "Broken" };

describe("ImagePreview placeholder (the image's colour and preview while it loads)", () => {
it("paints the colour and preview instead of a spinner while loading", () => {
const { container } = render(
<ImagePreview
size="tile"
src="/a.jpg"
alt=""
labels={labels}
placeholder={{ color: "#336699", lqip: "data:image/webp;base64,AAAA" }}
/>,
);
const frame = container.querySelector("[data-image-status]") as HTMLElement;
expect(frame.dataset.imageStatus).toBe("loading");
expect(frame.style.backgroundColor).toBe("#336699");
expect(frame.style.backgroundImage).toContain("data:image/webp;base64,AAAA");
expect(container.querySelector("[role=status]")?.textContent).toBe("Loading image");
});

it("keeps the spinner when there is no placeholder", () => {
const { container } = render(<ImagePreview size="tile" src="/a.jpg" alt="" labels={labels} />);
const frame = container.querySelector("[data-image-status]") as HTMLElement;
expect(frame.getAttribute("style")).toBeNull();
});
});
2 changes: 1 addition & 1 deletion scripts/size-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"apps/studio/src/routes/SchedulesPage.tsx": 1296,
"apps/studio/src/routes/SiteSettingsPage.tsx": 881,
"packages/api/src/build-turn.ts": 885,
"packages/api/src/routes/assets.ts": 853,
"packages/api/src/routes/assets.ts": 833,
"packages/api/src/routes/comments.ts": 874,
"packages/api/src/routes/schedules.ts": 992,
"packages/api/src/testing-roles.ts": 1936,
Expand Down
Loading