fix(webmail): a new profile photo shows up everywhere at once
The photo endpoint was cached for five minutes with no revalidation, and only the settings page added a version to the URL, so the header, the message list and the reader kept the old photo until it expired. - every <Avatar> reads a per-address version (and refetches when it changes); saving or removing a photo bumps it, in this tab and the other open tabs - the server answers private, max-age=60, must-revalidate, so a stale copy lives for a minute at most and then costs one 304 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d82cf71f5b
commit
c0f556b190
6 files changed
+146
-4
No files matched your search
@@ -946,6 +946,14 @@ describeDb("auth API against a real database", () => {
|
||||
expect(got.headers.get("content-type")).toBe("image/png");
|
||||
expect(Buffer.from(await got.arrayBuffer())).toEqual(png);
|
||||
|
||||
// A new photo has to show up quickly: the browser may reuse the old one
|
||||
// for at most a minute, and must ask again after that. It was five minutes
|
||||
// with no revalidation, which is what made a changed photo "take forever".
|
||||
const cacheControl = got.headers.get("cache-control") ?? "";
|
||||
expect(cacheControl).toContain("private");
|
||||
expect(cacheControl).toContain("must-revalidate");
|
||||
expect(Number(/max-age=(\d+)/.exec(cacheControl)?.[1])).toBeLessThanOrEqual(60);
|
||||
|
||||
// A photo appears beside every message from that person; without the
|
||||
// conditional answer it would be re-sent on every one of them.
|
||||
const etag = got.headers.get("etag")!;
|
||||
|
||||
@@ -343,7 +343,10 @@ async function getAvatar(
|
||||
res.setHeader("ETag", `"${avatar.etag}"`);
|
||||
// Private: this is served to a signed-in reader, and a shared cache must not
|
||||
// hand one person's photo to the next request that comes along.
|
||||
res.setHeader("Cache-Control", "private, max-age=300");
|
||||
// One minute, then revalidate with the ETag (a 304 costs no body). It was five
|
||||
// minutes with no revalidation, so a new photo took that long to show up
|
||||
// wherever the page did not add a version to the URL.
|
||||
res.setHeader("Cache-Control", "private, max-age=60, must-revalidate");
|
||||
// The bytes were sniffed on the way in, but a browser that ignores the type
|
||||
// and sniffs for itself must not be allowed to find something else.
|
||||
res.setHeader("X-Content-Type-Options", "nosniff");
|
||||
|
||||
@@ -26,6 +26,7 @@ import { signatureToHtml, sanitizeSignature } from "../../../../lib/signature";
|
||||
import { buildSieve, parseSieve, type Rule, type RuleField, type RuleCondition, type RuleAction } from "../../../../lib/rules";
|
||||
import { useFolders } from "../../../../lib/useFolders";
|
||||
import { AvatarEditor } from "../../../../components/AvatarEditor";
|
||||
import { bumpAvatar } from "../../../../lib/avatarVersion";
|
||||
import { useAccount } from "../../../../lib/useAccount";
|
||||
import { moveTargets } from "../../../../lib/mailCounts";
|
||||
|
||||
@@ -357,8 +358,10 @@ export default function UserSettingsPage() {
|
||||
src={avatarSrc}
|
||||
onSaved={() => {
|
||||
// The query string changes so the browser fetches the new
|
||||
// photo instead of the one it already has under this URL.
|
||||
// photo instead of the one it already has under this URL;
|
||||
// bumpAvatar does the same for every <Avatar> on screen.
|
||||
setAvatarVersion((v) => v + 1);
|
||||
bumpAvatar(account?.email);
|
||||
}}
|
||||
/>
|
||||
</section>
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
"use client";
|
||||
|
||||
import React, { useState } from "react";
|
||||
import React, { useEffect, useState, useSyncExternalStore } from "react";
|
||||
import { initialsFor } from "../lib/initials";
|
||||
import { avatarVersionOf, subscribeAvatarVersions } from "../lib/avatarVersion";
|
||||
|
||||
interface AvatarProps {
|
||||
address: string | null | undefined;
|
||||
@@ -28,7 +29,23 @@ export function Avatar({ address, size, className = "", initials }: AvatarProps)
|
||||
const [loaded, setLoaded] = useState(false);
|
||||
const [failed, setFailed] = useState(false);
|
||||
|
||||
const src = address ? `/api/v1/avatar?address=${encodeURIComponent(address)}` : null;
|
||||
// Bumped when this person's photo is saved or removed, here or in another
|
||||
// tab, so the browser asks again instead of reusing what it cached
|
||||
const version = useSyncExternalStore(
|
||||
subscribeAvatarVersions,
|
||||
() => avatarVersionOf(address),
|
||||
() => 0,
|
||||
);
|
||||
|
||||
const src = address
|
||||
? `/api/v1/avatar?address=${encodeURIComponent(address)}${version ? `&v=${version}` : ""}`
|
||||
: null;
|
||||
|
||||
// A new version may turn "no photo" (404) into a photo, or the reverse
|
||||
useEffect(() => {
|
||||
setLoaded(false);
|
||||
setFailed(false);
|
||||
}, [version]);
|
||||
|
||||
return (
|
||||
<span
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { avatarVersionOf, bumpAvatar, subscribeAvatarVersions } from "./avatarVersion";
|
||||
|
||||
describe("avatar versions", () => {
|
||||
it("starts at 0 for a photo nobody changed", () => {
|
||||
expect(avatarVersionOf("nobody@example.test")).toBe(0);
|
||||
expect(avatarVersionOf(null)).toBe(0);
|
||||
});
|
||||
|
||||
it("changes after a bump, whatever the case of the address", () => {
|
||||
bumpAvatar("Ana@Example.test", 1000);
|
||||
expect(avatarVersionOf("ana@example.test")).toBe(1000);
|
||||
expect(avatarVersionOf(" ANA@example.test ")).toBe(1000);
|
||||
});
|
||||
|
||||
it("never repeats a version, even when bumped in the same millisecond", () => {
|
||||
bumpAvatar("same@example.test", 5000);
|
||||
const first = avatarVersionOf("same@example.test");
|
||||
bumpAvatar("same@example.test", 5000);
|
||||
expect(avatarVersionOf("same@example.test")).toBeGreaterThan(first);
|
||||
});
|
||||
|
||||
it("does not touch other people's photos", () => {
|
||||
bumpAvatar("one@example.test", 7000);
|
||||
expect(avatarVersionOf("two@example.test")).toBe(0);
|
||||
});
|
||||
|
||||
it("tells subscribers, and stops when they unsubscribe", () => {
|
||||
const listener = vi.fn();
|
||||
const unsubscribe = subscribeAvatarVersions(listener);
|
||||
bumpAvatar("watch@example.test", 9000);
|
||||
expect(listener).toHaveBeenCalledTimes(1);
|
||||
unsubscribe();
|
||||
bumpAvatar("watch@example.test", 9001);
|
||||
expect(listener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("ignores an empty address", () => {
|
||||
const listener = vi.fn();
|
||||
const unsubscribe = subscribeAvatarVersions(listener);
|
||||
bumpAvatar("", 1);
|
||||
bumpAvatar(undefined, 1);
|
||||
expect(listener).not.toHaveBeenCalled();
|
||||
unsubscribe();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Which version of each person's photo this browser should ask for.
|
||||
*
|
||||
* The photo endpoint is cached by the browser, and the URL is the cache key.
|
||||
* Only the settings page used to add a version to it, so after a new photo was
|
||||
* saved the header, the message list and the reader kept showing the old one
|
||||
* from cache until it expired. Every <Avatar> now reads its version from here,
|
||||
* so one bump refetches the photo everywhere on screen, and in the other open
|
||||
* tabs of the same browser.
|
||||
*/
|
||||
|
||||
type Listener = () => void;
|
||||
|
||||
const CHANNEL = "lambdamail-avatar";
|
||||
|
||||
const versions = new Map<string, number>();
|
||||
const listeners = new Set<Listener>();
|
||||
let channel: BroadcastChannel | null = null;
|
||||
|
||||
function key(address: string | null | undefined): string {
|
||||
return (address ?? "").trim().toLowerCase();
|
||||
}
|
||||
|
||||
function openChannel(): BroadcastChannel | null {
|
||||
if (channel) return channel;
|
||||
if (typeof BroadcastChannel === "undefined") return null;
|
||||
channel = new BroadcastChannel(CHANNEL);
|
||||
channel.onmessage = (event: MessageEvent<{ address?: string; version?: number }>) => {
|
||||
const { address, version } = event.data ?? {};
|
||||
if (typeof address === "string" && typeof version === "number") {
|
||||
apply(address, version);
|
||||
}
|
||||
};
|
||||
return channel;
|
||||
}
|
||||
|
||||
function apply(address: string, version: number): void {
|
||||
const k = key(address);
|
||||
if (!k || (versions.get(k) ?? 0) >= version) return;
|
||||
versions.set(k, version);
|
||||
listeners.forEach((listener) => listener());
|
||||
}
|
||||
|
||||
/** 0 until the photo is known to have changed in this session. */
|
||||
export function avatarVersionOf(address: string | null | undefined): number {
|
||||
return versions.get(key(address)) ?? 0;
|
||||
}
|
||||
|
||||
/** Call after saving or removing a photo: everything showing it refetches. */
|
||||
export function bumpAvatar(address: string | null | undefined, now: number = Date.now()): void {
|
||||
const k = key(address);
|
||||
if (!k) return;
|
||||
// Strictly increasing, so two bumps in the same millisecond still differ
|
||||
const version = Math.max(now, (versions.get(k) ?? 0) + 1);
|
||||
apply(k, version);
|
||||
openChannel()?.postMessage({ address: k, version });
|
||||
}
|
||||
|
||||
export function subscribeAvatarVersions(listener: Listener): () => void {
|
||||
openChannel();
|
||||
listeners.add(listener);
|
||||
return () => {
|
||||
listeners.delete(listener);
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user