diff --git a/ui/src/Components/Project.jsx b/ui/src/Components/Project.jsx index b9f7ca37..203009d3 100644 --- a/ui/src/Components/Project.jsx +++ b/ui/src/Components/Project.jsx @@ -21,7 +21,7 @@ import { } from "@fluentui/react-components"; import { useParams } from "react-router-dom"; import { useState, useEffect } from "react"; -import { apiGet } from "../util/api"; +import { apiGet, apiGetResponse } from "../util/api"; import { useNavigate } from "react-router-dom"; import LayerRow from "./ProjectManagement/LayerRow"; @@ -37,7 +37,9 @@ import { updateUserSettings } from "../AppHelper"; import { collectProjectJobStates, findJobStatusTransitions, + hasActiveProjectJobs, } from "../util/jobNotifications"; +import { createSingleFlight } from "../util/singleFlight"; import PropType from "prop-types"; @@ -64,6 +66,7 @@ const GROUP_OPTIONS = [ ]; const PAGE_SIZE_OPTIONS = [5, 8, 10, 20, 50]; +const EMPTY_LAYERS = []; /** Resolve the group bucket label for an image layer given the grouping. */ function getLayerGroupLabel(item, mode) { @@ -133,13 +136,11 @@ const Project = ({ setModalComponent }) => { const defaultProjectDetailsRef = useRef(null); const projectJobStatesRef = useRef(null); + const projectLoadRef = useRef(createSingleFlight()); + const projectEtagRef = useRef(null); + const projectLifecycleRef = useRef(0); + const projectMountedRef = useRef(false); const { dispatchToast } = useToastController("job-completion-toaster"); - - - useEffect(() => { - setCurrentPage(1); - }, [appParams.userSettings.itemsPerPageLayers]); - const DEFAULT_COMPONENT_STATE = { project: null, visibleModelId: imageLayerId || "-1", @@ -148,41 +149,45 @@ const Project = ({ setModalComponent }) => { const projectCurrentTouruseRef = useRef(DEFAULT_COMPONENT_STATE.visibleModelId === "-1" ? "singleProjectGuide" : "singleProjectModelGuide"); - const [moreInfoVisibleId, setMoreInfoVisibleId] = useState(null); + const [, setMoreInfoVisibleId] = useState(null); const [componentState, setComponentState] = useState(DEFAULT_COMPONENT_STATE); const navigate = useNavigate(); useEffect(() => { - const fetchData = async () => { - await fetchProjectDetails(); - }; - fetchData(); + fetchProjectDetails(true, false); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [imageLayerId]); + }, [projectId, imageLayerId]); useEffect(() => { - const fetchData = async () => { - await fetchProjectDetails(); - - initGuidedTourState(projectCurrentTouruseRef.current, appParams.guidedTourProperties); - initCurrentTour(projectCurrentTouruseRef.current); - - setAppHeaderRightButtons([ - { - iconName: "help", - title: "Help", - id: "helpButton", - onClick: () => - setGuidedTourState(false, initCurrentTour, projectCurrentTouruseRef.current, appParams.guidedTourProperties), - }, - ]); - }; - fetchData(); + const lifecycle = ++projectLifecycleRef.current; + const projectLoad = projectLoadRef.current; + const initialTour = projectCurrentTouruseRef.current; + projectMountedRef.current = true; + initGuidedTourState(initialTour, appParams.guidedTourProperties); + initCurrentTour(initialTour); + + setAppHeaderRightButtons([ + { + iconName: "help", + title: "Help", + id: "helpButton", + onClick: () => + setGuidedTourState(false, initCurrentTour, projectCurrentTouruseRef.current, appParams.guidedTourProperties), + }, + ]); //On component dismount return () => { + projectMountedRef.current = false; + queueMicrotask(() => { + // StrictMode immediately advances this generation before the microtask. + // eslint-disable-next-line react-hooks/exhaustive-deps + if (projectLifecycleRef.current === lifecycle) { + projectLoad.abort(); + } + }); initCurrentTour(null); - initGuidedTourState(projectCurrentTouruseRef.current, appParams.guidedTourProperties); + initGuidedTourState(initialTour, appParams.guidedTourProperties); setAppHeaderRightButtons([]); setModalComponent(null); }; @@ -198,12 +203,38 @@ const Project = ({ setModalComponent }) => { } }, [componentState.visibleModelId]); - async function fetchProjectDetails(showLoading = true) { - if (showLoading) { + async function fetchProjectDetails( + showLoading = true, + forceRefresh = showLoading + ) { + if (projectEtagRef.current?.projectId !== projectId) { + projectEtagRef.current = null; + } + + const projectLoad = projectLoadRef.current; + if (forceRefresh && projectLoad.isRunning(projectId)) { + projectLoad.abort(); + } + const startsRequest = !projectLoad.isRunning(projectId); + if (showLoading && startsRequest) { setIsLoading(true); } - await apiGet("GetProjectDetails?projectId=" + projectId + "&includeModels=True") - .then((response) => { + const headers = {}; + if (forceRefresh) { + headers["Cache-Control"] = "no-cache"; + } + if (projectEtagRef.current?.etag) { + headers["If-None-Match"] = projectEtagRef.current.etag; + } + + const requestPromise = projectLoad.run(projectId, async (signal) => { + try { + const { data: response, etag, status } = await apiGetResponse( + "GetProjectDetails?projectId=" + projectId + "&includeModels=True", + { signal, headers, cache: forceRefresh ? "no-cache" : "default" } + ); + if (etag) projectEtagRef.current = { projectId, etag }; + if (status === 304) return defaultProjectDetailsRef.current; defaultProjectDetailsRef.current = response; const currentJobStates = collectProjectJobStates(response); const previousJobState = projectJobStatesRef.current; @@ -250,18 +281,33 @@ const Project = ({ setModalComponent }) => { filter: false, }, })); - }) - .catch((error) => { + return response; + } catch (error) { + if (error.name === "AbortError") return null; console.error("Error fetching projects:", error); + return null; + } + }); + if (showLoading && startsRequest) { + requestPromise.finally(() => { + if (!projectLoad.isRunning() && projectMountedRef.current) { + setIsLoading(false); + } }); - if (showLoading) { - setIsLoading(false); } + return requestPromise; } useEffect(() => { - const intervalId = setInterval(async () => { - fetchProjectDetails(false); + const intervalId = setInterval(() => { + const jobs = projectJobStatesRef.current?.jobs; + if ( + document.visibilityState === "visible" && + !projectLoadRef.current.isRunning() && + hasActiveProjectJobs(jobs) + ) { + fetchProjectDetails(false); + } }, 20000); return () => clearInterval(intervalId); @@ -307,7 +353,7 @@ const Project = ({ setModalComponent }) => { } // Filter + sort + group the image layers (memoised so pagination is cheap). - const imageLayers = componentState.project?.imageLayer || []; + const imageLayers = componentState.project?.imageLayer || EMPTY_LAYERS; const processed = useMemo(() => { const search = searchText.toLowerCase(); const filtered = imageLayers.filter( @@ -332,7 +378,6 @@ const Project = ({ setModalComponent }) => { return String(av ?? "").localeCompare(String(bv ?? "")) * dir; }); return sorted; - // eslint-disable-next-line react-hooks/exhaustive-deps }, [imageLayers, searchText, sort, effectiveGroupBy]); if (!componentState.project) { @@ -416,7 +461,7 @@ const Project = ({ setModalComponent }) => { { setModalComponent(null); - fetchProjectDetails(false); + fetchProjectDetails(false, true); }} projectId={projectId} /> diff --git a/ui/src/util/api.js b/ui/src/util/api.js index 62c10143..c5b2e759 100644 --- a/ui/src/util/api.js +++ b/ui/src/util/api.js @@ -5,11 +5,7 @@ const APIUrl = import.meta.env.VITE_API_URL; const APIMSubscriptionKey = import.meta.env.VITE_APIM_SUBSCRIPTION_KEY; import { upsertUser } from "../AppHelper.js"; import { sanitizeRedirectPath } from "./validation.js"; - -function resolveVarConcatChar(text) { - if (text === "") return ""; - return text.includes("?") ? "&" : "?"; -} +import { fetchJsonResponse } from "./http.js"; export function buildUrl(endpoint) { const base = APIUrl + endpoint; @@ -68,14 +64,15 @@ export async function apiLogout(redirectPath = "/") { } export async function apiGet(endpoint) { - try { - const response = await fetch(buildUrl(endpoint)); + const response = await apiGetResponse(endpoint); + return response.data; +} - if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - return await response.json(); +export async function apiGetResponse(endpoint, options = {}) { + try { + return await fetchJsonResponse(buildUrl(endpoint), options); } catch (error) { + if (error.name === "AbortError") throw error; console.error("Error fetching.:", error); throw new Error("Error fetching."); } @@ -137,7 +134,7 @@ export async function apiPost(endpoint, data, isFormData = false) { throw new Error(message.error || `HTTP error! status: ${response.status}`); } return await response.json(); - } catch (error) { + } catch { throw new Error("Error uploading chunk."); } } @@ -151,7 +148,7 @@ export async function apiDelete(endpoint) { throw new Error(`HTTP error! status: ${response.status}`); } return response; - } catch (error) { + } catch { throw new Error("Error deleting element."); } } \ No newline at end of file diff --git a/ui/src/util/http.js b/ui/src/util/http.js new file mode 100644 index 00000000..5ba7c917 --- /dev/null +++ b/ui/src/util/http.js @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export async function fetchJsonResponse(url, options = {}, fetchImpl = fetch) { + const response = await fetchImpl(url, options); + const etag = response.headers?.get?.("etag") ?? null; + + if (response.status === 304) { + return { data: null, etag, status: response.status }; + } + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + + const data = response.status === 204 ? null : await response.json(); + return { data, etag, status: response.status }; +} \ No newline at end of file diff --git a/ui/src/util/http.test.js b/ui/src/util/http.test.js new file mode 100644 index 00000000..27cd4056 --- /dev/null +++ b/ui/src/util/http.test.js @@ -0,0 +1,78 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { fetchJsonResponse } from "./http.js"; + +function response({ status = 200, data = null, etag = null } = {}) { + return { + status, + ok: status >= 200 && status < 300, + headers: { get: (name) => (name === "etag" ? etag : null) }, + json: async () => data, + }; +} + +test("returns parsed JSON and ETag", async () => { + const fetchImpl = async () => + response({ data: { projectId: "project-1" }, etag: '"etag"' }); + + const result = await fetchJsonResponse("/project", {}, fetchImpl); + + assert.deepEqual(result, { + data: { projectId: "project-1" }, + etag: '"etag"', + status: 200, + }); +}); + +test("returns an empty successful result for 304", async () => { + let jsonCalled = false; + const fetchImpl = async () => ({ + ...response({ status: 304, etag: '"etag"' }), + json: async () => { + jsonCalled = true; + }, + }); + + const result = await fetchJsonResponse("/project", {}, fetchImpl); + + assert.deepEqual(result, { data: null, etag: '"etag"', status: 304 }); + assert.equal(jsonCalled, false); +}); + +test("returns null for a successful empty response", async () => { + const result = await fetchJsonResponse( + "/project", + {}, + async () => response({ status: 204 }) + ); + + assert.deepEqual(result, { data: null, etag: null, status: 204 }); +}); + +test("rejects unsuccessful responses", async () => { + await assert.rejects( + fetchJsonResponse( + "/project", + {}, + async () => response({ status: 503 }) + ), + /status: 503/ + ); +}); + +test("passes request options to fetch", async () => { + const controller = new AbortController(); + const options = { + signal: controller.signal, + headers: { "If-None-Match": '"etag"' }, + }; + let receivedOptions; + + await fetchJsonResponse("/project", options, async (_url, received) => { + receivedOptions = received; + return response({ data: {} }); + }); + + assert.equal(receivedOptions, options); +}); \ No newline at end of file diff --git a/ui/src/util/jobNotifications.js b/ui/src/util/jobNotifications.js index 29050361..50e218eb 100644 --- a/ui/src/util/jobNotifications.js +++ b/ui/src/util/jobNotifications.js @@ -1,7 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -const TERMINAL_STATUSES = new Set(["Processed", "Failed", "Cancelled"]); +const TERMINAL_STATUSES = new Set([ + "Processed", + "Trained", + "Failed", + "Cancelled", +]); export function collectProjectJobStates(project) { const jobs = new Map(); @@ -54,4 +59,11 @@ export function findJobStatusTransitions(previousJobs, currentJobs) { } return transitions; +} + +export function hasActiveProjectJobs(jobs) { + if (!jobs) return false; + return [...jobs.values()].some( + (job) => job.status && !TERMINAL_STATUSES.has(job.status) + ); } \ No newline at end of file diff --git a/ui/src/util/jobNotifications.test.js b/ui/src/util/jobNotifications.test.js index 555075c9..73be59eb 100644 --- a/ui/src/util/jobNotifications.test.js +++ b/ui/src/util/jobNotifications.test.js @@ -4,6 +4,7 @@ import assert from "node:assert/strict"; import { collectProjectJobStates, findJobStatusTransitions, + hasActiveProjectJobs, } from "./jobNotifications.js"; function projectWithEmbeddingStatus(status) { @@ -95,4 +96,68 @@ test("reports terminal failures but ignores intermediate updates", () => { subject: "Fire model", }, ]); +}); + +test("detects whether project jobs still need polling", () => { + assert.equal( + hasActiveProjectJobs( + new Map([ + ["done", { status: "Processed" }], + ["active", { status: "InProgress" }], + ]) + ), + true + ); + assert.equal( + hasActiveProjectJobs( + new Map([ + ["done", { status: "Processed" }], + ["trained", { status: "Trained" }], + ["failed", { status: "Failed" }], + ["unknown", { status: null }], + ]) + ), + false + ); + assert.equal(hasActiveProjectJobs(null), false); +}); + +test("collecting jobs ignores malformed records and uses fallback names", () => { + assert.deepEqual([...collectProjectJobStates(null)], []); + assert.deepEqual([...collectProjectJobStates({})], []); + + const jobs = collectProjectJobStates({ + imageLayer: [ + { name: "Missing id", models: [] }, + { imageLayerId: "layer-without-models", status: "Processed" }, + { + imageLayerId: "layer-1", + models: [ + { status: "Queued" }, + { modelId: "model-1", status: "Queued" }, + { + modelId: "embedding-1", + modelType: "embedding", + status: "Queued", + inferenceStatus: "InProgress", + }, + ], + }, + ], + }); + + assert.deepEqual(jobs.get("imagery:layer-1").subject, "Image layer"); + assert.deepEqual(jobs.get("training:model-1").subject, "Model"); + assert.deepEqual(jobs.get("embedding:embedding-1").subject, "Embedding"); + assert.deepEqual(jobs.get("inference:embedding-1").subject, "Model"); +}); + +test("does not report unchanged or nonterminal job states", () => { + const previous = new Map([ + ["same", { status: "Processed" }], + ["active", { status: "Queued" }], + ]); + const current = new Map(previous); + + assert.deepEqual(findJobStatusTransitions(previous, current), []); }); \ No newline at end of file diff --git a/ui/src/util/singleFlight.js b/ui/src/util/singleFlight.js new file mode 100644 index 00000000..25a9e0f8 --- /dev/null +++ b/ui/src/util/singleFlight.js @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export function createSingleFlight() { + let active = null; + + return { + run(key, task) { + if (active?.key === key) return active.promise; + active?.controller.abort(); + + const controller = new AbortController(); + const entry = { controller, key, promise: null }; + entry.promise = Promise.resolve() + .then(() => task(controller.signal)) + .finally(() => { + if (active === entry) active = null; + }); + active = entry; + return entry.promise; + }, + + abort() { + active?.controller.abort(); + active = null; + }, + + isRunning(key) { + return active !== null && (key === undefined || active.key === key); + }, + }; +} \ No newline at end of file diff --git a/ui/src/util/singleFlight.test.js b/ui/src/util/singleFlight.test.js new file mode 100644 index 00000000..10fb5b6b --- /dev/null +++ b/ui/src/util/singleFlight.test.js @@ -0,0 +1,98 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { createSingleFlight } from "./singleFlight.js"; + +test("deduplicates concurrent work for the same key", async () => { + const flight = createSingleFlight(); + let calls = 0; + let release; + const pending = new Promise((resolve) => { + release = resolve; + }); + const task = async () => { + calls += 1; + await pending; + return "value"; + }; + + const first = flight.run("project-1", task); + const second = flight.run("project-1", task); + release(); + + assert.equal(first, second); + assert.equal(await first, "value"); + assert.equal(calls, 1); + assert.equal(flight.isRunning(), false); +}); + +test("starting a different key aborts the previous task", async () => { + const flight = createSingleFlight(); + let firstSignal; + const first = flight.run("project-1", async (signal) => { + firstSignal = signal; + await new Promise((resolve) => signal.addEventListener("abort", resolve)); + return "aborted"; + }); + await Promise.resolve(); + + const second = flight.run("project-2", async () => "current"); + + assert.equal(firstSignal.aborted, true); + assert.equal(await first, "aborted"); + assert.equal(await second, "current"); +}); + +test("failed work clears the flight so it can be retried", async () => { + const flight = createSingleFlight(); + + await assert.rejects( + flight.run("project-1", async () => { + throw new Error("failed"); + }), + /failed/ + ); + assert.equal(flight.isRunning("project-1"), false); + assert.equal( + await flight.run("project-1", async () => "recovered"), + "recovered" + ); +}); + +test("abort signals and clears active work", async () => { + const flight = createSingleFlight(); + let signal; + const pending = flight.run("project-1", async (currentSignal) => { + signal = currentSignal; + await new Promise((resolve) => + currentSignal.addEventListener("abort", resolve) + ); + }); + await Promise.resolve(); + + flight.abort(); + + assert.equal(signal.aborted, true); + assert.equal(flight.isRunning(), false); + await pending; +}); + +test("reports running state by key and tolerates idle abort", async () => { + const flight = createSingleFlight(); + let release; + const pending = flight.run( + "project-1", + () => new Promise((resolve) => { + release = resolve; + }) + ); + await Promise.resolve(); + + assert.equal(flight.isRunning(), true); + assert.equal(flight.isRunning("project-1"), true); + assert.equal(flight.isRunning("project-2"), false); + release(); + await pending; + flight.abort(); + assert.equal(flight.isRunning(), false); +}); \ No newline at end of file