From 6f27069ce4e50b956703ef2cb9bda3bd55770010 Mon Sep 17 00:00:00 2001 From: Cameron Dawson Date: Sun, 5 Jul 2026 10:11:29 -0700 Subject: [PATCH] Cache Similar Jobs tab with stale-while-revalidate The Similar Jobs tab is unmounted by react-tabs when inactive and remounted on return, which reset the list, pagination, selected job, and filter and refetched everything (page 1 + push data + the auto-selected job's detail). Switching back felt like a full reload and lost the user's place. Add a per-session snapshot cache keyed by the investigated jobId and restore it on mount, then stale-while-revalidate: - Restore the full tab state instantly (list, loaded pages, selected job and its detail, filter) with no spinner. - If the snapshot's list was fetched more than 30s ago, revalidate in the background: refetch all loaded pages in one request and refresh the selected job's detail, then swap the fresh data in. This picks up newly-arrived same-type jobs and status changes on already-listed jobs. - A 30s dedup window avoids refetching on rapid tab toggling. - A mount guard lets a revalidate that resolves after the user tabs away update the cache while skipping setState. - Bounded to 50 jobIds, evicting the oldest. The tab's fetch logic is refactored into reusable fetchAndEnrich (page range) and fetchJobDetail helpers. The left job-info panel is unaffected: the selected job's own status and log-parse status still update live via useJobDetails and the push poll. Add unit tests for the cache (store/evict/recency/freshness) and the tab (cache-miss fetch, fresh-hit dedup, stale-hit background revalidate). --- tests/ui/job-view/SimilarJobsTab_test.jsx | 157 ++++++++++ tests/ui/job-view/similarJobsCache_test.js | 69 +++++ ui/job-view/details/tabs/SimilarJobsTab.jsx | 296 ++++++++++++++----- ui/job-view/details/tabs/similarJobsCache.js | 36 +++ 4 files changed, 487 insertions(+), 71 deletions(-) create mode 100644 tests/ui/job-view/SimilarJobsTab_test.jsx create mode 100644 tests/ui/job-view/similarJobsCache_test.js create mode 100644 ui/job-view/details/tabs/similarJobsCache.js diff --git a/tests/ui/job-view/SimilarJobsTab_test.jsx b/tests/ui/job-view/SimilarJobsTab_test.jsx new file mode 100644 index 00000000000..22f0f89bb7e --- /dev/null +++ b/tests/ui/job-view/SimilarJobsTab_test.jsx @@ -0,0 +1,157 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router'; + +import SimilarJobsTab from '../../../ui/job-view/details/tabs/SimilarJobsTab'; +import JobModel from '../../../ui/models/job'; +import PushModel from '../../../ui/models/push'; +import { + similarJobsCache, + setSimilarJobsSnapshot, + SIMILAR_JOBS_REVALIDATE_MS, +} from '../../../ui/job-view/details/tabs/similarJobsCache'; + +jest.mock('../../../ui/models/job', () => ({ + __esModule: true, + default: { getSimilarJobs: jest.fn(), get: jest.fn() }, +})); +jest.mock('../../../ui/models/push', () => ({ + __esModule: true, + default: { getList: jest.fn() }, +})); +jest.mock('../../../ui/helpers/http', () => ({ + getData: jest.fn(() => Promise.resolve({ data: [], failureStatus: null })), +})); + +// A job-detail object with the fields addAggregateFields()/getResultState() need. +const detailJob = (id) => ({ + id, + job_group_name: 'group', + job_type_name: 'reftest', + job_type_symbol: 'R2', + platform: 'linux', + platform_option: 'opt', + submit_timestamp: 1600000000, + start_timestamp: 1600000000, + end_timestamp: 1600000060, + duration: 4, + state: 'completed', + result: 'success', + failure_classification_id: 1, +}); + +const JOB_ID = 500; +const classificationMap = { 1: { star: 'label-default', name: 'unclassified' } }; + +// A fully enriched similar-job row, as it lives in a cached snapshot. +const enrichedJob = (id, symbol = 'R1') => ({ + id, + push_id: 5, + job_type_symbol: symbol, + failure_classification_id: 1, + resultStatus: 'success', + duration: 3, + result_set: { + push_timestamp: 1600000000, + author: 'dev@example.com', + revisions: [{ revision: 'abc123def456' }], + }, + revisionResultsetFilterUrl: '#rev', + authorResultsetFilterUrl: '#author', +}); + +const renderTab = () => + render( + + + , + ); + +describe('SimilarJobsTab caching', () => { + beforeEach(() => { + similarJobsCache.clear(); + JobModel.getSimilarJobs.mockReset(); + JobModel.get.mockReset(); + JobModel.get.mockResolvedValue(detailJob(601)); + PushModel.getList.mockReset(); + // Default fetch responses for the cache-miss / revalidate paths. + JobModel.getSimilarJobs.mockResolvedValue({ + data: [ + { + id: 601, + push_id: 5, + job_type_symbol: 'R2', + failure_classification_id: 1, + resultStatus: 'success', + duration: 4, + }, + ], + failureStatus: null, + }); + PushModel.getList.mockResolvedValue({ + data: { + results: [ + { + id: 5, + push_timestamp: 1600000000, + author: 'dev@example.com', + revisions: [{ revision: 'abc123def456' }], + }, + ], + }, + failureStatus: null, + }); + }); + + test('fetches on a cache miss and renders the results', async () => { + renderTab(); + + await waitFor(() => + expect(JobModel.getSimilarJobs).toHaveBeenCalledTimes(1), + ); + expect(await screen.findByText('R2')).toBeInTheDocument(); + }); + + test('hydrates from a fresh snapshot without revalidating (dedup)', async () => { + setSimilarJobsSnapshot(JOB_ID, { + similarJobs: [enrichedJob(701, 'CACHED')], + page: 1, + filterNoSuccessfulJobs: false, + selectedSimilarJob: null, + hasNextPage: false, + timestamp: Date.now(), // fresh + }); + + renderTab(); + + // Restored instantly from cache... + expect(await screen.findByText('CACHED')).toBeInTheDocument(); + // ...and, being fresh, no background revalidate fires. + await waitFor(() => {}); + expect(JobModel.getSimilarJobs).not.toHaveBeenCalled(); + }); + + test('hydrates from a stale snapshot and revalidates in the background', async () => { + setSimilarJobsSnapshot(JOB_ID, { + similarJobs: [enrichedJob(801, 'STALE')], + page: 1, + filterNoSuccessfulJobs: false, + selectedSimilarJob: null, + hasNextPage: false, + timestamp: Date.now() - SIMILAR_JOBS_REVALIDATE_MS - 1, // stale + }); + + renderTab(); + + // Cached row shows immediately. + expect(await screen.findByText('STALE')).toBeInTheDocument(); + // Stale -> background revalidate fetches fresh data and swaps it in. + await waitFor(() => + expect(JobModel.getSimilarJobs).toHaveBeenCalledTimes(1), + ); + expect(await screen.findByText('R2')).toBeInTheDocument(); + }); +}); diff --git a/tests/ui/job-view/similarJobsCache_test.js b/tests/ui/job-view/similarJobsCache_test.js new file mode 100644 index 00000000000..2a2c6a53179 --- /dev/null +++ b/tests/ui/job-view/similarJobsCache_test.js @@ -0,0 +1,69 @@ +import { + similarJobsCache, + SIMILAR_JOBS_CACHE_LIMIT, + SIMILAR_JOBS_REVALIDATE_MS, + getSimilarJobsSnapshot, + setSimilarJobsSnapshot, + isSnapshotFresh, +} from '../../../ui/job-view/details/tabs/similarJobsCache'; + +const snapshot = (overrides = {}) => ({ + similarJobs: [{ id: 1 }], + page: 1, + filterNoSuccessfulJobs: false, + selectedSimilarJob: null, + hasNextPage: false, + timestamp: Date.now(), + ...overrides, +}); + +describe('similarJobsCache', () => { + afterEach(() => { + similarJobsCache.clear(); + }); + + test('stores and retrieves a snapshot by jobId', () => { + const snap = snapshot(); + setSimilarJobsSnapshot(42, snap); + + expect(getSimilarJobsSnapshot(42)).toBe(snap); + expect(getSimilarJobsSnapshot(99)).toBeNull(); + }); + + test('evicts the oldest jobId once the limit is exceeded', () => { + for (let jobId = 1; jobId <= SIMILAR_JOBS_CACHE_LIMIT; jobId++) { + setSimilarJobsSnapshot(jobId, snapshot()); + } + expect(similarJobsCache.size).toBe(SIMILAR_JOBS_CACHE_LIMIT); + expect(getSimilarJobsSnapshot(1)).not.toBeNull(); + + setSimilarJobsSnapshot(SIMILAR_JOBS_CACHE_LIMIT + 1, snapshot()); + + expect(similarJobsCache.size).toBe(SIMILAR_JOBS_CACHE_LIMIT); + expect(getSimilarJobsSnapshot(1)).toBeNull(); + expect(getSimilarJobsSnapshot(SIMILAR_JOBS_CACHE_LIMIT + 1)).not.toBeNull(); + }); + + test('re-writing a jobId refreshes its eviction recency', () => { + for (let jobId = 1; jobId <= SIMILAR_JOBS_CACHE_LIMIT; jobId++) { + setSimilarJobsSnapshot(jobId, snapshot()); + } + // Touch jobId 1 so it is no longer the oldest. + setSimilarJobsSnapshot(1, snapshot()); + // Next insert should evict jobId 2 (now the oldest), not 1. + setSimilarJobsSnapshot(SIMILAR_JOBS_CACHE_LIMIT + 1, snapshot()); + + expect(getSimilarJobsSnapshot(1)).not.toBeNull(); + expect(getSimilarJobsSnapshot(2)).toBeNull(); + }); + + test('isSnapshotFresh reflects the revalidate window', () => { + expect(isSnapshotFresh(null)).toBe(false); + expect(isSnapshotFresh(snapshot({ timestamp: Date.now() }))).toBe(true); + expect( + isSnapshotFresh( + snapshot({ timestamp: Date.now() - SIMILAR_JOBS_REVALIDATE_MS - 1 }), + ), + ).toBe(false); + }); +}); diff --git a/ui/job-view/details/tabs/SimilarJobsTab.jsx b/ui/job-view/details/tabs/SimilarJobsTab.jsx index 74a0e373cd8..bc8bf39f6db 100644 --- a/ui/job-view/details/tabs/SimilarJobsTab.jsx +++ b/ui/job-view/details/tabs/SimilarJobsTab.jsx @@ -14,6 +14,12 @@ import { notify } from '../../../shared/stores/notificationStore'; import { getProjectJobUrl } from '../../../helpers/location'; import { getData } from '../../../helpers/http'; +import { + getSimilarJobsSnapshot, + setSimilarJobsSnapshot, + isSnapshotFresh, +} from './similarJobsCache'; + const PAGE_SIZE = 20; function SimilarJobsTab({ repoName, classificationMap, selectedJobFull }) { @@ -22,15 +28,89 @@ function SimilarJobsTab({ repoName, classificationMap, selectedJobFull }) { const [page, setPage] = useState(1); const [selectedSimilarJob, setSelectedSimilarJob] = useState(null); const [hasNextPage, setHasNextPage] = useState(false); - const [isLoading, setIsLoading] = useState(true); + // Start without a spinner when we already have a snapshot to restore, so the + // cache hit renders instantly. The mount effect below manages it thereafter. + const [isLoading, setIsLoading] = useState( + () => !getSimilarJobsSnapshot(selectedJobFull.id), + ); + + // Tracks whether the component is still mounted, so a background revalidate + // that resolves after the user tabs away updates the cache but skips setState. + const isMountedRef = useRef(true); + // Time of the last successful *list* fetch, used as the cache snapshot's + // freshness timestamp (selection/pagination writes preserve it). + const lastFetchTimeRef = useRef(0); + + useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + }; + }, []); + + // Fetch a slice of similar jobs and enrich each with its push data. Returns + // { jobs, hasNext } or null on error. `wanted` is the number of rows to + // display; we request one extra to detect a further page. + const fetchAndEnrich = useCallback( + async ({ offset, wanted, currentFilter }) => { + const options = { count: wanted + 1, offset }; + if (currentFilter) { + options.nosuccess = ''; + } + + const { data: raw, failureStatus } = await JobModel.getSimilarJobs( + selectedJobFull.id, + options, + ); + if (failureStatus) { + notify(`Error fetching similar jobs: ${failureStatus}`, 'danger', { + sticky: true, + }); + return null; + } + + const hasNext = raw.length > wanted; + const jobs = hasNext ? raw.slice(0, wanted) : raw; + + const pushIds = [...new Set(jobs.map((job) => job.push_id))]; + const { data: pushData, failureStatus: pushFailureStatus } = + await PushModel.getList({ + id__in: pushIds.join(','), + count: thMaxPushFetchSize, + }); + if (pushFailureStatus) { + notify(`Error fetching similar jobs push data: ${pushData}`, 'danger', { + sticky: true, + }); + return null; + } - const similarJobsRef = useRef(similarJobs); - const selectedSimilarJobRef = useRef(selectedSimilarJob); - similarJobsRef.current = similarJobs; - selectedSimilarJobRef.current = selectedSimilarJob; + const pushes = pushData.results.reduce( + (acc, push) => ({ ...acc, [push.id]: push }), + {}, + ); + jobs.forEach((simJob) => { + simJob.result_set = pushes[simJob.push_id]; + simJob.revisionResultsetFilterUrl = getJobsUrl({ + repo: repoName, + revision: simJob.result_set.revisions[0].revision, + }); + simJob.authorResultsetFilterUrl = getJobsUrl({ + repo: repoName, + author: simJob.result_set.author, + }); + }); - const showJobInfo = useCallback((job) => { - JobModel.get(repoName, job.id).then(async (nextJob) => { + return { jobs, hasNext }; + }, + [selectedJobFull.id, repoName], + ); + + // Fetch a single similar job's detail (classification + error lines) for the + // detail panel. Returns the enriched job. + const fetchJobDetail = useCallback( + async (job) => { + const nextJob = await JobModel.get(repoName, job.id); addAggregateFields(nextJob); nextJob.failure_classification = classificationMap[nextJob.failure_classification_id]; @@ -41,90 +121,164 @@ function SimilarJobsTab({ repoName, classificationMap, selectedJobFull }) { if (!failureStatus && data.length) { nextJob.error_lines = data; } - setSelectedSimilarJob(nextJob); - }); - }, [repoName, classificationMap]); - - const getSimilarJobs = useCallback(async (currentPage, currentFilterNoSuccess) => { - const options = { - count: PAGE_SIZE + 1, - offset: (currentPage - 1) * PAGE_SIZE, - }; + return nextJob; + }, + [repoName, classificationMap], + ); - if (currentFilterNoSuccess) { - options.nosuccess = ''; - } + const showJobInfo = useCallback( + (job) => { + fetchJobDetail(job).then((nextJob) => { + if (isMountedRef.current) { + setSelectedSimilarJob(nextJob); + } + }); + }, + [fetchJobDetail], + ); - const { - data: newSimilarJobs, - failureStatus, - } = await JobModel.getSimilarJobs(selectedJobFull.id, options); + // Load the first page from scratch (cache miss, or filter change). + const loadInitial = useCallback( + async (currentFilter) => { + setIsLoading(true); + const result = await fetchAndEnrich({ + offset: 0, + wanted: PAGE_SIZE, + currentFilter, + }); + if (!isMountedRef.current) return; + setIsLoading(false); + if (!result) return; - if (!failureStatus) { - const nextPage = newSimilarJobs.length > PAGE_SIZE; - setHasNextPage(nextPage); - if (nextPage) { - newSimilarJobs.pop(); + lastFetchTimeRef.current = Date.now(); + setPage(1); + setSimilarJobs(result.jobs); + setHasNextPage(result.hasNext); + if (result.jobs.length > 0) { + showJobInfo(result.jobs[0]); } - const pushIds = [...new Set(newSimilarJobs.map((job) => job.push_id))]; - let pushList = { results: [] }; - const { data, failureStatus: pushFailureStatus } = await PushModel.getList({ - id__in: pushIds.join(','), - count: thMaxPushFetchSize, + }, + [fetchAndEnrich, showJobInfo], + ); + + // Stale-while-revalidate: refetch every currently-loaded page in one request, + // refresh the selected job's detail, then update the cache (always) and the + // component state (only if still mounted). + const revalidate = useCallback( + async (snapshot) => { + const { + page: snapPage, + filterNoSuccessfulJobs: snapFilter, + selectedSimilarJob: snapSelected, + } = snapshot; + + const result = await fetchAndEnrich({ + offset: 0, + wanted: snapPage * PAGE_SIZE, + currentFilter: snapFilter, }); + if (!result) return; + lastFetchTimeRef.current = Date.now(); - if (!pushFailureStatus) { - pushList = data; - const pushes = pushList.results.reduce( - (acc, push) => ({ ...acc, [push.id]: push }), - {}, + let refreshedSelected = snapSelected; + if (snapSelected && result.jobs.some((job) => job.id === snapSelected.id)) { + refreshedSelected = await fetchJobDetail(snapSelected).catch( + () => snapSelected, ); - newSimilarJobs.forEach((simJob) => { - simJob.result_set = pushes[simJob.push_id]; - simJob.revisionResultsetFilterUrl = getJobsUrl({ - repo: repoName, - revision: simJob.result_set.revisions[0].revision, - }); - simJob.authorResultsetFilterUrl = getJobsUrl({ - repo: repoName, - author: simJob.result_set.author, - }); - }); - setSimilarJobs((prev) => [...prev, ...newSimilarJobs]); - if (!selectedSimilarJobRef.current && newSimilarJobs.length > 0) { - showJobInfo(newSimilarJobs[0]); - } - } else { - notify(`Error fetching similar jobs push data: ${data}`, 'danger', { - sticky: true, - }); } - } else { - notify(`Error fetching similar jobs: ${failureStatus}`, 'danger', { - sticky: true, + + // Always refresh the cache so the next tab open is up to date, even if the + // user has already tabbed away. + setSimilarJobsSnapshot(selectedJobFull.id, { + similarJobs: result.jobs, + page: snapPage, + filterNoSuccessfulJobs: snapFilter, + selectedSimilarJob: refreshedSelected, + hasNextPage: result.hasNext, + timestamp: lastFetchTimeRef.current, }); + + if (isMountedRef.current) { + setSimilarJobs(result.jobs); + setHasNextPage(result.hasNext); + setSelectedSimilarJob(refreshedSelected); + } + }, + [fetchAndEnrich, fetchJobDetail, selectedJobFull.id], + ); + + // On mount (or when the investigated job changes): restore from cache and + // revalidate if stale, otherwise load fresh. + useEffect(() => { + const snapshot = getSimilarJobsSnapshot(selectedJobFull.id); + if (snapshot) { + setSimilarJobs(snapshot.similarJobs); + setPage(snapshot.page); + setFilterNoSuccessfulJobs(snapshot.filterNoSuccessfulJobs); + setSelectedSimilarJob(snapshot.selectedSimilarJob); + setHasNextPage(snapshot.hasNextPage); + setIsLoading(false); + lastFetchTimeRef.current = snapshot.timestamp; + if (!isSnapshotFresh(snapshot)) { + revalidate(snapshot); + } + } else { + loadInitial(false); } - setIsLoading(false); - }, [selectedJobFull.id, repoName, showJobInfo]); + // Intentionally keyed only on the investigated job id; loadInitial/revalidate + // are stable per job and we do not want to re-run on their identity changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectedJobFull.id]); + // Persist the tab state so a remount restores exactly where the user left off. + // Uses the last *fetch* time (not now) so the revalidate dedup stays accurate. useEffect(() => { - getSimilarJobs(1, false); - }, [getSimilarJobs]); + if (isLoading || similarJobs.length === 0) { + return; + } + setSimilarJobsSnapshot(selectedJobFull.id, { + similarJobs, + page, + filterNoSuccessfulJobs, + selectedSimilarJob, + hasNextPage, + timestamp: lastFetchTimeRef.current, + }); + }, [ + similarJobs, + page, + filterNoSuccessfulJobs, + selectedSimilarJob, + hasNextPage, + isLoading, + selectedJobFull.id, + ]); - const showNext = useCallback(() => { + const showNext = useCallback(async () => { const nextPage = page + 1; - setPage(nextPage); setIsLoading(true); - getSimilarJobs(nextPage, filterNoSuccessfulJobs); - }, [page, filterNoSuccessfulJobs, getSimilarJobs]); + const result = await fetchAndEnrich({ + offset: page * PAGE_SIZE, + wanted: PAGE_SIZE, + currentFilter: filterNoSuccessfulJobs, + }); + if (!isMountedRef.current) return; + setIsLoading(false); + if (!result) return; + + lastFetchTimeRef.current = Date.now(); + setPage(nextPage); + setSimilarJobs((prev) => [...prev, ...result.jobs]); + setHasNextPage(result.hasNext); + }, [page, filterNoSuccessfulJobs, fetchAndEnrich]); const toggleFilter = useCallback(() => { const newValue = !filterNoSuccessfulJobs; setFilterNoSuccessfulJobs(newValue); + setSelectedSimilarJob(null); setSimilarJobs([]); - setIsLoading(true); - getSimilarJobs(1, newValue); - }, [filterNoSuccessfulJobs, getSimilarJobs]); + loadInitial(newValue); + }, [filterNoSuccessfulJobs, loadInitial]); const selectedSimilarJobId = selectedSimilarJob ? selectedSimilarJob.id diff --git a/ui/job-view/details/tabs/similarJobsCache.js b/ui/job-view/details/tabs/similarJobsCache.js new file mode 100644 index 00000000000..cd760e971a7 --- /dev/null +++ b/ui/job-view/details/tabs/similarJobsCache.js @@ -0,0 +1,36 @@ +// Per-session cache of the Similar Jobs tab's full state, keyed by the +// investigated jobId. react-tabs unmounts the tab when inactive and remounts it +// on return, which would otherwise reset the list, pagination, selection and +// filter and refetch everything. We snapshot that state for an instant restore, +// then stale-while-revalidate in the background to pick up newly-arrived jobs +// and status changes on already-listed jobs. +// +// A snapshot's `timestamp` is the time of the last successful *list* fetch (not +// the last state change), so the revalidate dedup window below is meaningful. +export const similarJobsCache = new Map(); +export const SIMILAR_JOBS_CACHE_LIMIT = 50; +// Skip the background revalidate if the snapshot's list was fetched within this +// window, so rapid tab toggling doesn't spam the backend. +export const SIMILAR_JOBS_REVALIDATE_MS = 30 * 1000; + +export function getSimilarJobsSnapshot(jobId) { + return similarJobsCache.get(jobId) || null; +} + +export function setSimilarJobsSnapshot(jobId, snapshot) { + // Re-insert so this jobId counts as the newest for eviction ordering (Map + // preserves insertion order). + similarJobsCache.delete(jobId); + if (similarJobsCache.size >= SIMILAR_JOBS_CACHE_LIMIT) { + similarJobsCache.delete(similarJobsCache.keys().next().value); + } + similarJobsCache.set(jobId, snapshot); +} + +// True when the snapshot's list is recent enough to skip a background +// revalidate. +export function isSnapshotFresh(snapshot) { + return ( + !!snapshot && Date.now() - snapshot.timestamp < SIMILAR_JOBS_REVALIDATE_MS + ); +}