refactor(ui): only re-fetch no-store artwork stand-ins

Refresh events also name albums and songs for plays, stars and ratings, so
versioning every cached cover re-downloaded final, immutable images on each of
them. useImageUrl now marks a no-store response as a stand-in and re-fetches only
those entries, on remount as well as on a version change; final images are never
re-fetched, and cache entries no longer carry a version.

useArtworkRefresh now selects a primitive, so an event naming other items does
not re-render every mounted cover.

ui/src/common/useArtworkRefresh.js

@@ -9,15 +9,15 @@ const namesId = (resources, id) =>
 // Artwork can change behind an unchanged URL (a background conversion replacing its stand-in), so
 // this returns when the server last announced new artwork for id, for useImageUrl to version by.
 export const useArtworkRefresh = (id) => {
-  const refresh = useSelector((state) => state.activity?.refresh)
+  // A primitive, so an event naming other items does not re-render this cover.
+  const named = useSelector(({ activity }) =>
+    id && namesId(activity?.refresh?.resources, id)
+      ? activity.refresh.lastReceived
+      : undefined,
+  )
   const [refreshedAt, setRefreshedAt] = useState()
-  if (
-    id &&
-    refresh &&
-    namesId(refresh.resources, id) &&
-    refresh.lastReceived !== refreshedAt
-  ) {
-    setRefreshedAt(refresh.lastReceived)
+  if (named !== undefined && named !== refreshedAt) {
+    setRefreshedAt(named)
   }
   return refreshedAt
 }

ui/src/common/useImageUrl.js

@@ -34,8 +34,8 @@ const evictIfNeeded = () => {
 /**
  * Loads an image via fetch() with AbortController so that in-flight requests
  * are canceled on unmount (e.g., during pagination). Uses a module-level cache
- * so remounting returns the cached blob URL instantly. A new version re-fetches a cached url whose
- * image changed behind it, painting the old blob until the new one arrives.
+ * so remounting returns the cached blob URL instantly. A no-store response is a stand-in: it is
+ * re-fetched on remount and on a new version, and painted until its replacement arrives.
  */
 export const useImageUrl = (url, version) => {
   const cached = url ? cache.get(url) : null
@@ -69,7 +69,7 @@ export const useImageUrl = (url, version) => {
     // Re-check: another component's effect may have populated the cache
     // between this component's render and effect execution.
     const entry = cache.get(url)
-    if (entry && entry.version === version) {
+    if (entry && !entry.transient) {
       entry.refCount++
       setImgUrl(entry.blobUrl)
       setLoading(false)
@@ -78,11 +78,11 @@ export const useImageUrl = (url, version) => {
         entry.refCount--
       }
     }
-    const stale = entry?.blobUrl ? entry : null
+    const standIn = !!entry
 
     const controller = new AbortController()
     let queued = true
-    if (!stale) {
+    if (!standIn) {
       setImgUrl(null)
       setLoading(true)
       setError(false)
@@ -96,9 +96,12 @@ export const useImageUrl = (url, version) => {
           if (!res.ok) {
             throw new Error(`HTTP ${res.status}`)
           }
-          return res.blob()
+          const transient = !!res.headers
+            ?.get('Cache-Control')
+            ?.includes('no-store')
+          return res.blob().then((blob) => ({ blob, transient }))
         })
-        .then((blob) => {
+        .then(({ blob, transient }) => {
           activeFetches--
           processQueue()
           // Guard against late resolution after abort
@@ -109,7 +112,7 @@ export const useImageUrl = (url, version) => {
           // Handle concurrent fetches: if another component already cached
           // this URL, use its entry and discard our blob.
           const existing = cache.get(url)
-          if (existing && existing.blobUrl && existing.version === version) {
+          if (existing?.blobUrl && !existing.transient) {
             existing.refCount++
             URL.revokeObjectURL(objectUrl)
             setImgUrl(existing.blobUrl)
@@ -117,7 +120,7 @@ export const useImageUrl = (url, version) => {
             if (existing?.blobUrl && existing.refCount <= 0) {
               URL.revokeObjectURL(existing.blobUrl)
             }
-            cache.set(url, { blobUrl: objectUrl, refCount: 1, version })
+            cache.set(url, { blobUrl: objectUrl, refCount: 1, transient })
             evictIfNeeded()
             setImgUrl(objectUrl)
           }
@@ -129,11 +132,11 @@ export const useImageUrl = (url, version) => {
           if (err.name === 'AbortError') {
             return // Expected on unmount or URL change
           }
-          if (stale) {
-            return // The old image is still valid; keep showing it
+          if (standIn) {
+            return // Keep painting the stand-in
           }
           // Cache the error so repeated mounts don't re-fetch broken URLs
-          cache.set(url, { blobUrl: null, error: true, refCount: 0, version })
+          cache.set(url, { blobUrl: null, error: true, refCount: 0 })
           setError(true)
           setLoading(false)
         })

ui/src/common/useImageUrl.test.js

@@ -235,89 +235,97 @@ describe('useImageUrl', () => {
     expect(result2.current.fromCache).toBe(false)
   })
 
-  describe('version', () => {
-    const ok = (data) =>
+  describe('no-store stand-ins', () => {
+    const image = (data, cacheControl = 'public, no-cache') =>
       Promise.resolve({
         ok: true,
+        headers: {
+          get: (name) => (name === 'Cache-Control' ? cacheControl : null),
+        },
         blob: () => Promise.resolve(new Blob([data])),
       })
+    const standIn = (data) => image(data, 'no-store')
     const renderVersioned = (version) =>
       renderHook(
         ({ version }) => useImageUrl('http://example.com/img.jpg', version),
         { initialProps: { version } },
       )
+    const settle = () =>
+      act(async () => {
+        await flushPromises()
+      })
 
-    it('re-fetches a cached url when its version changes, showing the old image meanwhile', async () => {
-      let resolveNew
+    it('re-fetches a stand-in when its version changes, painting it meanwhile', async () => {
+      let resolveFinal
       global.URL.createObjectURL = vi
         .fn()
-        .mockReturnValueOnce('blob:old')
-        .mockReturnValueOnce('blob:new')
+        .mockReturnValueOnce('blob:stand-in')
+        .mockReturnValueOnce('blob:final')
       global.fetch = vi
         .fn()
-        .mockImplementationOnce(() => ok('old'))
+        .mockImplementationOnce(() => standIn('still'))
         .mockImplementationOnce(
-          () => new Promise((resolve) => (resolveNew = resolve)),
+          () => new Promise((resolve) => (resolveFinal = resolve)),
         )
 
       const { result, rerender } = renderVersioned(undefined)
-      await act(async () => {
-        await flushPromises()
-      })
-      expect(result.current.imgUrl).toBe('blob:old')
+      await settle()
+      expect(result.current.imgUrl).toBe('blob:stand-in')
 
       rerender({ version: 1 })
-      await act(async () => {
-        await flushPromises()
-      })
+      await settle()
       expect(global.fetch).toHaveBeenCalledTimes(2)
-      expect(result.current.imgUrl).toBe('blob:old')
+      expect(result.current.imgUrl).toBe('blob:stand-in')
       expect(result.current.loading).toBe(false)
 
       await act(async () => {
-        resolveNew({ ok: true, blob: () => Promise.resolve(new Blob(['new'])) })
+        resolveFinal(image('animated'))
         await flushPromises()
       })
-      expect(result.current.imgUrl).toBe('blob:new')
+      expect(result.current.imgUrl).toBe('blob:final')
     })
 
-    it('keeps the old image when the re-fetch fails', async () => {
+    it('keeps the stand-in when the re-fetch fails', async () => {
       global.fetch = vi
         .fn()
-        .mockImplementationOnce(() => ok('old'))
+        .mockImplementationOnce(() => standIn('still'))
         .mockImplementationOnce(() =>
           Promise.resolve({ ok: false, status: 500 }),
         )
 
       const { result, rerender } = renderVersioned(undefined)
-      await act(async () => {
-        await flushPromises()
-      })
+      await settle()
       rerender({ version: 1 })
-      await act(async () => {
-        await flushPromises()
-      })
+      await settle()
 
       expect(global.fetch).toHaveBeenCalledTimes(2)
       expect(result.current.imgUrl).toBe('blob:mock-url')
       expect(result.current.error).toBe(false)
     })
 
-    it('does not re-fetch on remount with the version it already has', async () => {
-      global.fetch = vi.fn(() => ok('data'))
+    // Plays, stars and ratings also name items, and must not re-download final images.
+    it('does not re-fetch a final image when its version changes', async () => {
+      global.fetch = vi.fn(() => image('final'))
 
-      const { unmount } = renderVersioned(1)
-      await act(async () => {
-        await flushPromises()
-      })
-      unmount()
-      const { result } = renderVersioned(1)
-      await act(async () => {
-        await flushPromises()
-      })
+      const { rerender } = renderVersioned(undefined)
+      await settle()
+      rerender({ version: 1 })
+      await settle()
 
       expect(global.fetch).toHaveBeenCalledTimes(1)
-      expect(result.current.fromCache).toBe(true)
+    })
+
+    it('re-fetches a stand-in on remount', async () => {
+      global.fetch = vi.fn(() => standIn('still'))
+
+      const { unmount } = renderVersioned(undefined)
+      await settle()
+      unmount()
+      const { result } = renderVersioned(undefined)
+      await settle()
+
+      expect(global.fetch).toHaveBeenCalledTimes(2)
+      expect(result.current.imgUrl).toBe('blob:mock-url')
     })
   })
 })