From e3015a3b2bb5683a1cba89374af1ac0d97c6b500 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 21:46:30 +0000 Subject: [PATCH] Web UI: label a Docker group's manifest times by their member's kind On a group's image page, each manifest's time is what its member's is: "Cached" for one from a proxy member, "Pushed" for one from a hosted member. The members' kinds come from /browse/repos, which lists every member a group serves the reader. When the rows mix kinds, or a member's kind is not known, the column is "Pushed / cached" and each row of a known member says which its time is. A group's image list merges its members' images and names none of them, so its column says "Last pushed / cached". Fixes #562 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJYfDiGC4npKLCFPnZL --- docs/changes/2.2.0/562.fixed.md | 6 ++ web/src/components/ImageTables.tsx | 78 +++++++++++++++++--- web/src/routes/Browse.test.tsx | 36 +++++++++- web/src/routes/ImageDetail.test.tsx | 108 ++++++++++++++++++++++++++++ web/src/routes/RepoBrowse.tsx | 11 ++- 5 files changed, 228 insertions(+), 11 deletions(-) create mode 100644 docs/changes/2.2.0/562.fixed.md diff --git a/docs/changes/2.2.0/562.fixed.md b/docs/changes/2.2.0/562.fixed.md new file mode 100644 index 00000000..c62d6797 --- /dev/null +++ b/docs/changes/2.2.0/562.fixed.md @@ -0,0 +1,6 @@ +A Docker group's pages labelled the times of manifests served by a proxy +member "Pushed", though they are when Forge cached them from the upstream. +An image's manifests now say "Cached" or "Pushed" by the member each comes +from, and "Pushed / cached" with a label on each row where the members mix; +a group's image list, which names no member, heads its column "Last pushed / +cached". diff --git a/web/src/components/ImageTables.tsx b/web/src/components/ImageTables.tsx index dc100534..5f3c3b2a 100644 --- a/web/src/components/ImageTables.tsx +++ b/web/src/components/ImageTables.tsx @@ -1,4 +1,7 @@ +import { useQuery } from '@tanstack/react-query' + import type { ImageManifest, ImageTag } from '~/api/images' +import { browseReposQueryOptions } from '~/api/packages' import { formatBytes, formatDateTime, @@ -37,6 +40,14 @@ import { * Forge first cached it from the upstream, not when it was pushed there, so * the column is labelled "Cached" for a proxy repository (as package * versions are, #110). + * + * A group's manifests come from its members (`source`), and each row's time + * is what its member's is: cached for a proxy member, pushed for a hosted one + * (#562). The members' kinds come from the readable repositories + * (`/browse/repos`), which list every member a group serves the reader. When + * every row has the same kind the column is labelled for it; when the rows + * mix kinds, or a row's member is not known, the column is "Pushed / cached" + * and each row of a known member says which its time is. */ /** A short digest with its full value as tooltip, plus a copy button. */ @@ -136,6 +147,37 @@ export function TagTable({ ) } +/** What a manifest's `created_at` is: when it was pushed, or first cached. */ +type TimeKind = 'pushed' | 'cached' + +/** The time kind of a repository (or a group's member) of `kind`. */ +function timeKindOf(kind: string | undefined): TimeKind | undefined { + if (kind === 'proxy') return 'cached' + if (kind === 'hosted') return 'pushed' + return undefined +} + +const TIME_HEADERS: Record = { + pushed: { label: 'Pushed' }, + cached: { + label: 'Cached', + hint: 'When Forge first cached this manifest from the upstream', + }, + mixed: { + label: 'Pushed / cached', + hint: 'When the manifest was pushed to a hosted member, or first cached from the upstream by a proxy member', + }, +} + +/** + * The kind of the member each manifest of a group comes from, by member + * name; empty for any other repository (no request is made). + */ +function useMemberKinds(repoKind: string): Map { + const repos = useQuery({ ...browseReposQueryOptions, enabled: repoKind === 'group' }) + return new Map((repoKind === 'group' ? (repos.data ?? []) : []).map((r) => [r.name, r.kind])) +} + const KIND_ICONS: Record = { Image: 'Package', Index: 'Layers', @@ -160,6 +202,18 @@ export function ManifestTable({ // Referrers name their subject by digest; show the subject's tags too. const tagsByDigest = new Map(manifests.map((m) => [m.digest, m.tags])) const showSource = repoKind === 'group' + const memberKinds = useMemberKinds(repoKind) + const rowTimeKind = (m: ImageManifest): TimeKind | undefined => + timeKindOf(showSource ? m.source && memberKinds.get(m.source) : repoKind) + const rowKinds = new Set(manifests.map(rowTimeKind)) + // One kind for every row labels the column for it; a mix (or a member of + // unknown kind) gets the neutral heading and a per-row label. + const single: TimeKind | undefined = showSource + ? rowKinds.size === 1 + ? [...rowKinds][0] + : undefined + : (timeKindOf(repoKind) ?? 'pushed') + const timeHeader = TIME_HEADERS[single ?? 'mixed'] return ( @@ -169,13 +223,7 @@ export function ManifestTable({ SizePlatformsTags - {repoKind === 'proxy' ? ( - - Cached - - ) : ( - Pushed - )} + {timeHeader.label}Last pulled {showSource && Source} {onDelete && Actions} @@ -199,7 +247,7 @@ export function ManifestTable({ - {formatDateTime(m.created_at)} + {formatDateTime(m.last_pulled_at)} @@ -223,6 +271,20 @@ export function ManifestTable({ ) } +/** + * A manifest's `created_at`; in a column of mixed kinds, with whether the + * row's time is a push or a cache. + */ +function TimeCell({ manifest, label }: { manifest: ImageManifest; label?: TimeKind }) { + if (!label) return <>{formatDateTime(manifest.created_at)} + return ( + + {formatDateTime(manifest.created_at)} + {label === 'cached' ? 'Cached' : 'Pushed'} + + ) +} + /** * The manifest's kind badge (media / artifact type as tooltip); an index * adds how many manifests it lists, a referrer (signature, attestation, SBOM, diff --git a/web/src/routes/Browse.test.tsx b/web/src/routes/Browse.test.tsx index ee3259db..ea8e984b 100644 --- a/web/src/routes/Browse.test.tsx +++ b/web/src/routes/Browse.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { render, screen, waitFor, within } from '@testing-library/react' +import { cleanup, render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { @@ -115,6 +115,7 @@ function rowOf(element: HTMLElement): HTMLElement { function mockApi( browse: (url: URL) => Response, images: (url: URL) => Response = () => jsonResponse(imagePage(0, [])), + repoList: BrowseRepo[] = repos, ) { fetchMock.mockImplementation((input: RequestInfo | URL) => { const raw = typeof input === 'string' ? input : input.toString() @@ -123,7 +124,7 @@ function mockApi( return Promise.resolve(jsonResponse(readerMe)) } if (url.pathname.endsWith('/browse/repos')) { - return Promise.resolve(jsonResponse(repos)) + return Promise.resolve(jsonResponse(repoList)) } if (url.pathname.endsWith('/browse/packages')) { return Promise.resolve(browse(url)) @@ -606,6 +607,37 @@ describe('RepoBrowse of a docker repository (images)', () => { } }) + it('labels a proxy\'s and a group\'s image times by what they are (#248, #562)', async () => { + const dockerRepos: BrowseRepo[] = [ + ...repos, + { id: 6, name: 'docker-hub', kind: 'proxy', online: true, format: 'docker' }, + { id: 7, name: 'docker-all', kind: 'group', online: true, format: 'docker' }, + ] + const images = () => jsonResponse(imagePage(1, ['team/app'])) + mockApi(() => jsonResponse(page(0, [])), images, dockerRepos) + + await renderBrowse('/repos/docker-hub') + + await screen.findByRole('link', { name: /team\/app/ }) + expect(screen.getByRole('columnheader', { name: 'Last cached' })).toBeInTheDocument() + expect(screen.queryByRole('columnheader', { name: /pushed/i })).toBeNull() + + cleanup() + await renderBrowse('/repos/docker-all') + + // A group's image rows merge its hosted and proxy members' and name + // none of them: the column says both. + await screen.findByRole('link', { name: /team\/app/ }) + expect( + screen.getByRole('columnheader', { name: 'Last pushed / cached' }), + ).toHaveAttribute( + 'title', + 'When a manifest of the image was last pushed to a hosted member, or cached from the upstream by a proxy member', + ) + expect(screen.queryByRole('columnheader', { name: 'Last pushed' })).toBeNull() + expect(screen.queryByRole('columnheader', { name: 'Last cached' })).toBeNull() + }) + it('says a docker repository is offline on its 503', async () => { mockApi( () => jsonResponse(page(0, [])), diff --git a/web/src/routes/ImageDetail.test.tsx b/web/src/routes/ImageDetail.test.tsx index 5309e840..344acee6 100644 --- a/web/src/routes/ImageDetail.test.tsx +++ b/web/src/routes/ImageDetail.test.tsx @@ -11,6 +11,7 @@ import { import { clearSession } from '~/lib/auth' import type { Me } from '~/api/me' import type { ChartDetail, ImageDetail } from '~/api/images' +import type { BrowseRepo } from '~/api/packages' import { textContent } from '~/test/text' import { routeTree } from '~/routeTree.gen' import { searchOptions } from '~/lib/search' @@ -121,6 +122,7 @@ const fetchMock = vi.fn() /** * Route the mock: `/admin/me` -> identity (`me`, a reader by default), + * `/browse/repos` -> `repos`, * `/browse/images/{repo}/{image…}`, `/browse/chart` -> `chart` (404 without * one), and a `DELETE /admin/repositories/{repo}/images` -> `deleteImage`. */ @@ -130,9 +132,12 @@ function mockApi( { me = readerMe, deleteImage, + repos, }: { me?: Me deleteImage?: (repo: string, query: URLSearchParams) => Response | Promise + /** `/browse/repos` (404 without). */ + repos?: BrowseRepo[] } = {}, ) { fetchMock.mockImplementation((input: RequestInfo | URL, init?: RequestInit) => { @@ -145,6 +150,9 @@ function mockApi( if (del && init?.method === 'DELETE' && deleteImage) { return Promise.resolve(deleteImage(decodeURIComponent(del[1]), url.searchParams)) } + if (url.pathname.endsWith('/browse/repos') && repos) { + return Promise.resolve(jsonResponse(repos)) + } if (url.pathname.endsWith('/browse/chart')) { return Promise.resolve( chart ? chart(url.searchParams) : new Response('chart not found', { status: 404 }), @@ -483,6 +491,106 @@ describe('ImageDetail', () => { expect(within(manifests).getAllByText('docker-hosted')).toHaveLength(4) }) + describe('labels a group\'s manifest times by the member they come from (#562)', () => { + const members: BrowseRepo[] = [ + { id: 1, name: 'docker-hosted', kind: 'hosted', online: true, format: 'docker' }, + { id: 2, name: 'docker-hub', kind: 'proxy', online: true, format: 'docker' }, + { id: 3, name: 'docker-group', kind: 'group', online: true, format: 'docker' }, + ] + const groupDetail = (sources: string[]) => ({ + ...hostedDetail, + repo: 'docker-group', + repo_kind: 'group', + pull_name: 'forge.example.com/docker-group/team/app', + manifests: hostedDetail.manifests.map((m, i) => ({ ...m, source: sources[i] })), + }) + /** The time cell of the manifest row whose digest starts `short`. */ + const timeOf = (manifests: HTMLElement, short: string) => { + const header = within(manifests).getByRole('columnheader', { name: /Pushed|Cached/ }) + const column = Array.from(header.parentElement!.children).indexOf(header) + const row = within(manifests) + .getByText(short, { selector: '.fg-digest-short' }) + .closest('tr') as HTMLElement + return within(row).getAllByRole('cell')[column] + } + + it('says per row whether a mixed group\'s time is a push or a cache', async () => { + // The index and its amd64 image from the proxy, the rest pushed. + mockApi( + () => + jsonResponse( + groupDetail(['docker-hub', 'docker-hub', 'docker-hosted', 'docker-hosted']), + ), + undefined, + { repos: members }, + ) + + await renderDetail('/images/docker-group/team/app') + + const manifests = await screen.findByRole('region', { name: 'Manifests' }) + await waitFor(() => + expect(within(manifests).getAllByText('Cached', { selector: '.fg-cell-sub' })).toHaveLength(2), + ) + expect( + within(manifests).getByRole('columnheader', { name: 'Pushed / cached' }), + ).toHaveAttribute( + 'title', + 'When the manifest was pushed to a hosted member, or first cached from the upstream by a proxy member', + ) + expect(within(manifests).queryByRole('columnheader', { name: 'Pushed' })).toBeNull() + expect(within(manifests).queryByRole('columnheader', { name: 'Cached' })).toBeNull() + expect(timeOf(manifests, '1111111111aa')).toHaveTextContent(/Cached$/) + expect(timeOf(manifests, '2222222222bb')).toHaveTextContent(/Cached$/) + expect(timeOf(manifests, '4444444444dd')).toHaveTextContent(/Pushed$/) + expect( + within(manifests).getAllByText('Pushed', { selector: '.fg-cell-sub' }), + ).toHaveLength(2) + }) + + it('labels the column "Cached" when every manifest comes from a proxy member', async () => { + mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hub'))), undefined, { + repos: members, + }) + + await renderDetail('/images/docker-group/team/app') + + const manifests = await screen.findByRole('region', { name: 'Manifests' }) + expect( + await within(manifests).findByRole('columnheader', { name: 'Cached' }), + ).toHaveAttribute('title', 'When Forge first cached this manifest from the upstream') + expect(within(manifests).queryByRole('columnheader', { name: /Pushed/ })).toBeNull() + // One kind for the column: no per-row label. + expect(within(manifests).queryByText('Cached', { selector: '.fg-cell-sub' })).toBeNull() + }) + + it('labels the column "Pushed" when every manifest comes from a hosted member', async () => { + mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hosted'))), undefined, { + repos: members, + }) + + await renderDetail('/images/docker-group/team/app') + + const manifests = await screen.findByRole('region', { name: 'Manifests' }) + expect( + await within(manifests).findByRole('columnheader', { name: 'Pushed' }), + ).toBeInTheDocument() + expect(within(manifests).queryByRole('columnheader', { name: /Cached/ })).toBeNull() + }) + + it('keeps the neutral heading while a member\'s kind is not known', async () => { + // No `/browse/repos`: the members' kinds are unknown. + mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hub')))) + + await renderDetail('/images/docker-group/team/app') + + const manifests = await screen.findByRole('region', { name: 'Manifests' }) + expect( + within(manifests).getByRole('columnheader', { name: 'Pushed / cached' }), + ).toBeInTheDocument() + expect(within(manifests).queryByText(/^(Pushed|Cached)$/, { selector: '.fg-cell-sub' })).toBeNull() + }) + }) + it('pulls an untagged image by the digest of its top-level manifest', async () => { mockApi(() => jsonResponse({ diff --git a/web/src/routes/RepoBrowse.tsx b/web/src/routes/RepoBrowse.tsx index 2f9101ff..adfdeef6 100644 --- a/web/src/routes/RepoBrowse.tsx +++ b/web/src/routes/RepoBrowse.tsx @@ -571,7 +571,9 @@ function ImageBrowseResults({ Manifests - {/* A proxy's images carry their cache time, not a push (#248). */} + {/* A proxy's images carry their cache time, not a push (#248). + A group's merge its members' and name none of them, so its + column says both (#562). */} {repoKind === 'proxy' ? ( Last cached + ) : repoKind === 'group' ? ( + + Last pushed / cached + ) : ( Last pushed )}