fix: show a re-uploaded logo or icon at once instead of after a restart

app/api/admin/branding/route.ts

@@ -12,6 +12,7 @@ import { logger } from '@/lib/logger';
 import path from 'node:path';
 import { configReadOnlyResponse } from '@/lib/admin/config-read-only-response';
 import { brandingDocs } from '@/lib/admin/extension-store';
+import type { PutResult } from '@/lib/shared-store';
 import { announceChange } from '@/lib/shared-store/changes';
 
 // Uploads live in C/branding, or the shared store in shared mode (#1066),
@@ -111,6 +112,10 @@ function clearDomainKeys(
   return next;
 }
 
+function brandingUrl(name: string, written: PutResult): string {
+  return `/api/admin/branding/${name}${written.ok ? `?v=${written.version}` : ''}`;
+}
+
 /**
  * POST /api/admin/branding - Upload a branding image file
  *
@@ -177,9 +182,12 @@ export async function POST(request: NextRequest) {
     }
 
     const buffer = Buffer.from(await file.arrayBuffer());
-    await docs.put(safeName, buffer);
+    const written = await docs.put(safeName, buffer);
 
-    const servedUrl = `/api/admin/branding/${safeName}`;
+    // The version in the URL changes with the bytes, so browsers (which may
+    // keep the file for an hour) and the rendered icons and previews pick up
+    // a re-upload of the same slot and type at once.
+    const servedUrl = brandingUrl(safeName, written);
     await configManager.ensureLoaded();
 
     if (host) {

app/api/og-image/route.ts

@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server';
 import sharp from 'sharp';
 import { configManager } from '@/lib/admin/config-manager';
 import { fetchBrandingAsset } from '@/lib/admin/branding-asset';
+import { renderCache } from '@/lib/admin/render-cache';
 import {
   matchDomainBranding,
   parseDomainBranding,
@@ -63,7 +64,9 @@ function isDarkColor({ r, g, b }: Rgb): boolean {
 
 // Cache rendered cards keyed by (source logo, background) so a branding change
 // invalidates the previous render instead of serving stale bytes forever.
-const cache = new Map<string, Blob>();
+// An uploaded logo's URL carries its version. Capped, and dropped on a
+// branding change.
+const cache = renderCache('og-image');
 
 export async function GET(req: NextRequest) {
   await configManager.ensureLoaded();

app/api/pwa-icon/[size]/route.ts

@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server';
 import sharp from 'sharp';
 import { configManager } from '@/lib/admin/config-manager';
 import { fetchBrandingAsset } from '@/lib/admin/branding-asset';
+import { renderCache } from '@/lib/admin/render-cache';
 import {
   matchDomainBranding,
   parseDomainBranding,
@@ -10,9 +11,11 @@ import {
 
 const VALID_SIZES = new Set([192, 512]);
 
-// Cache resized images keyed by (size, source URL) so admin re-uploads or URL
-// changes invalidate the prior render instead of serving stale bytes forever.
-const cache = new Map<string, Blob>();
+// Cache resized images keyed by (size, source URL) so admin re-uploads (the
+// URL carries the upload's version) or URL changes invalidate the prior
+// render instead of serving stale bytes forever. Capped, and dropped on a
+// branding change.
+const cache = renderCache('pwa-icon');
 
 export async function GET(
   req: NextRequest,

app/api/pwa-screenshot/[variant]/route.ts

@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server';
 import sharp from 'sharp';
 import { configManager } from '@/lib/admin/config-manager';
 import { fetchBrandingAsset } from '@/lib/admin/branding-asset';
+import { renderCache } from '@/lib/admin/render-cache';
 import {
   matchDomainBranding,
   parseDomainBranding,
@@ -20,8 +21,9 @@ const VARIANTS = {
 
 type Variant = keyof typeof VARIANTS;
 
-// Cache resized images keyed by (variant, source URL).
-const cache = new Map<string, Blob>();
+// Cache resized images keyed by (variant, source URL); an uploaded source's
+// URL carries its version. Capped, and dropped on a branding change.
+const cache = renderCache('pwa-screenshot');
 
 export async function GET(
   req: NextRequest,

app/api/setup/branding/route.ts

@@ -102,9 +102,10 @@ export async function POST(request: NextRequest) {
     }
 
     const buffer = Buffer.from(await file.arrayBuffer());
-    await docs.put(safeName, buffer);
+    const written = await docs.put(safeName, buffer);
 
-    const servedUrl = `/api/admin/branding/${safeName}`;
+    // Versioned like the admin upload's URL (app/api/admin/branding/route.ts).
+    const servedUrl = `/api/admin/branding/${safeName}${written.ok ? `?v=${written.version}` : ''}`;
     await configManager.ensureLoaded();
     await configManager.setAdminConfig({ [slot]: servedUrl });
     await announceChange('branding');

lib/admin/__tests__/branding-reupload.test.ts

@@ -0,0 +1,136 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { mkdtemp, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import sharp from 'sharp';
+import { NextRequest } from 'next/server';
+
+// A re-upload of the same branding slot with the same file type kept its
+// URL. Browsers kept showing the old file for up to an hour, and the PWA
+// icon, screenshot and link-preview renders, cached by that URL, kept the
+// old logo until the server restarted. The URL now carries the upload's
+// version, and the render caches are capped and dropped on a branding or
+// config change (also one made on another instance, #1066).
+
+vi.mock('@/lib/admin/session', () => ({
+  requireAdminAuth: async () => ({ payload: { role: 'admin' } }),
+  getClientIP: () => '127.0.0.1',
+}));
+vi.mock('@/lib/admin/audit', () => ({ auditLog: async () => {} }));
+
+import { configManager } from '../config-manager';
+import { POST as upload } from '@/app/api/admin/branding/route';
+import { GET as pwaIcon } from '@/app/api/pwa-icon/[size]/route';
+import { GET as serveFile } from '@/app/api/admin/branding/[filename]/route';
+import { MAX_ENTRIES, clearRenderCaches, renderCache } from '../render-cache';
+import { setStoreForTests } from '@/lib/shared-store';
+import { resetLocalStoreForTests } from '@/lib/shared-store/local';
+import { INIT_EPOCH, markEpochsSeen, pollChanges, resetChangesForTests } from '@/lib/shared-store/changes';
+import { separateFakeStore } from './shared-store-fake';
+
+let dir: string;
+
+const square = (r: number, g: number, b: number) =>
+  sharp({ create: { width: 64, height: 64, channels: 4, background: { r, g, b, alpha: 1 } } }).png().toBuffer();
+
+async function uploadIcon(png: Buffer): Promise<string> {
+  const form = new FormData();
+  form.set('file', new File([new Uint8Array(png)], 'icon.png', { type: 'image/png' }));
+  form.set('slot', 'pwaIconUrl');
+  const res = await upload(new NextRequest('https://mail.example/api/admin/branding', { method: 'POST', body: form }));
+  expect(res.status).toBe(200);
+  return (await res.json()).url;
+}
+
+async function renderedIconColour(route = pwaIcon): Promise<number[]> {
+  const res = await route(new NextRequest('https://mail.example/api/pwa-icon/192'), { params: Promise.resolve({ size: '192' }) });
+  const { data } = await sharp(Buffer.from(await res.arrayBuffer())).raw().toBuffer({ resolveWithObject: true });
+  // The centre pixel.
+  const at = (96 * 192 + 96) * 4;
+  return [data[at], data[at + 1], data[at + 2]];
+}
+
+beforeEach(async () => {
+  dir = await mkdtemp(path.join(tmpdir(), 'bw-branding-reupload-'));
+  vi.stubEnv('ADMIN_CONFIG_DIR', path.join(dir, 'config'));
+  vi.stubEnv('ADMIN_STATE_DIR', path.join(dir, 'state'));
+  resetLocalStoreForTests();
+  resetChangesForTests();
+  clearRenderCaches();
+  await configManager.load();
+});
+
+afterEach(async () => {
+  setStoreForTests(null);
+  resetChangesForTests();
+  resetLocalStoreForTests();
+  vi.unstubAllEnvs();
+  await rm(dir, { recursive: true, force: true });
+});
+
+describe('re-uploading a branding image', () => {
+  it('gets a new URL, and the rendered PWA icon shows the new image at once', async () => {
+    const first = await uploadIcon(await square(255, 0, 0));
+    expect(first).toMatch(/^\/api\/admin\/branding\/pwaIconUrl\.png\?v=[\w-]+$/);
+    expect(await renderedIconColour()).toEqual([255, 0, 0]);
+
+    const second = await uploadIcon(await square(0, 0, 255));
+    expect(second).not.toBe(first);
+    expect(second.split('?')[0]).toBe(first.split('?')[0]);
+    expect(configManager.get('pwaIconUrl')).toBe(second);
+    expect(await renderedIconColour()).toEqual([0, 0, 255]);
+
+    // The file itself is served under its name, whatever the version says.
+    const res = await serveFile(new NextRequest(`https://mail.example${second}`), { params: Promise.resolve({ filename: 'pwaIconUrl.png' }) });
+    expect(res.status).toBe(200);
+  });
+
+  it('renders anew after a branding change even when the URL stayed the same', async () => {
+    // The route as loaded at process start, listening for changes.
+    vi.resetModules();
+    const { GET: route } = await import('@/app/api/pwa-icon/[size]/route');
+    await uploadIcon(await square(255, 0, 0));
+    expect(await renderedIconColour(route)).toEqual([255, 0, 0]);
+    // The file behind the URL changes (as a linked remote logo can) and the
+    // branding change is announced.
+    const { brandingDocs } = await import('../extension-store');
+    await brandingDocs().put('pwaIconUrl.png', await square(0, 255, 0));
+    const { announceChange } = await import('@/lib/shared-store/changes');
+    await announceChange('branding');
+    expect(await renderedIconColour(route)).toEqual([0, 255, 0]);
+  });
+
+  it('keeps the same URL for the same bytes', async () => {
+    const png = await square(0, 255, 0);
+    expect(await uploadIcon(png)).toBe(await uploadIcon(png));
+  });
+});
+
+describe('render caches', () => {
+  it('hold at most a fixed number of images, dropping the least recently used', () => {
+    const cache = renderCache('test-cap');
+    for (let i = 0; i < MAX_ENTRIES; i++) cache.set(`k${i}`, new Blob([String(i)]));
+    expect(cache.get('k0')).toBeDefined(); // k0 used again
+    for (let i = MAX_ENTRIES; i < MAX_ENTRIES + 8; i++) cache.set(`k${i}`, new Blob([String(i)]));
+    expect(cache.size).toBe(MAX_ENTRIES);
+    expect(cache.has('k0')).toBe(true);
+    expect(cache.has('k1')).toBe(false);
+  });
+
+  it('are dropped when another instance changes the branding', async () => {
+    const store = separateFakeStore(path.join(dir, 'store'));
+    setStoreForTests(store);
+    await store.epochs.bump(INIT_EPOCH);
+    await markEpochsSeen();
+    // A fresh copy of the module listens, as each route bundle's does.
+    vi.resetModules();
+    const fresh = await import('../render-cache');
+    const cache = fresh.renderCache('og-image');
+    cache.set('logo', new Blob(['old render']));
+
+    await store.epochs.bump('branding');
+    await pollChanges();
+    expect(cache.has('logo')).toBe(false);
+  });
+});

lib/admin/branding-asset.ts

@@ -57,7 +57,8 @@ export async function fetchBrandingAsset(url: string, label = 'branding asset'):
   }
 
   if (url.startsWith(ADMIN_BRANDING_PREFIX)) {
-    const filename = path.basename(url.slice(ADMIN_BRANDING_PREFIX.length));
+    // The URL carries the upload's version (?v=…) for caches; the file is the path.
+    const filename = path.basename(url.slice(ADMIN_BRANDING_PREFIX.length).split(/[?#]/)[0]);
     const doc = await brandingDocs().get(filename);
     if (!doc) throw new Error(`Branding file ${filename} not found`);
     return doc.data;

lib/admin/render-cache.ts

@@ -0,0 +1,66 @@
+import { onSharedChange } from '@/lib/shared-store/changes';
+
+/**
+ * Images rendered from the branding (og-image, pwa-icon, pwa-screenshot),
+ * kept so each is made once. Keyed by the source URL, which for an upload
+ * carries the file's version (`?v=`), so a re-upload renders anew. Each
+ * cache holds at most MAX_ENTRIES (the least recently used goes first; a
+ * deployment with many domains used to grow it without bound), and all of
+ * them are dropped when the branding or config changes, on this instance or
+ * another one (#1066).
+ *
+ * On globalThis: one set of caches per process, whichever module copy asks.
+ */
+
+export const MAX_ENTRIES = 32;
+
+export interface RenderCache {
+  has(key: string): boolean;
+  get(key: string): Blob | undefined;
+  set(key: string, value: Blob): void;
+  readonly size: number;
+}
+
+const KEY = Symbol.for('bulwark.admin.renderCaches');
+
+function caches(): Map<string, Map<string, Blob>> {
+  const g = globalThis as typeof globalThis & { [KEY]?: Map<string, Map<string, Blob>> };
+  return (g[KEY] ??= new Map());
+}
+
+export function clearRenderCaches(): void {
+  for (const cache of caches().values()) cache.clear();
+}
+
+for (const area of ['branding', 'config']) onSharedChange(area, () => clearRenderCaches());
+
+export function renderCache(name: string): RenderCache {
+  const entries = () => {
+    const all = caches();
+    let cache = all.get(name);
+    if (!cache) all.set(name, (cache = new Map()));
+    return cache;
+  };
+  return {
+    has: (key) => entries().has(key),
+    get(key) {
+      const cache = entries();
+      const value = cache.get(key);
+      if (value !== undefined) {
+        // Most recently used last.
+        cache.delete(key);
+        cache.set(key, value);
+      }
+      return value;
+    },
+    set(key, value) {
+      const cache = entries();
+      cache.delete(key);
+      cache.set(key, value);
+      while (cache.size > MAX_ENTRIES) cache.delete(cache.keys().next().value!);
+    },
+    get size() {
+      return entries().size;
+    },
+  };
+}