diff --git a/client/src/components/History/CurrentCollection/CollectionPanel.vue b/client/src/components/History/CurrentCollection/CollectionPanel.vue index 2f58d1356e2a..10c830ca4601 100644 --- a/client/src/components/History/CurrentCollection/CollectionPanel.vue +++ b/client/src/components/History/CurrentCollection/CollectionPanel.vue @@ -43,7 +43,16 @@ const dsc = computed(() => { } return currentCollection; }); -const collectionElements = computed(() => collectionElementsStore.getCollectionElements(dsc.value, offset.value)); + +watch( + () => [dsc.value, offset.value], + () => { + collectionElementsStore.fetchMissingElements(dsc.value, offset.value); + }, + { immediate: true } +); + +const collectionElements = computed(() => collectionElementsStore.getCollectionElements(dsc.value) ?? []); const loading = computed(() => collectionElementsStore.isLoadingCollectionElements(dsc.value)); const jobState = computed(() => ("job_state_summary" in dsc.value ? dsc.value.job_state_summary : undefined)); const populatedStateMsg = computed(() => @@ -99,7 +108,8 @@ watch( watch( jobState, () => { - collectionElementsStore.loadCollectionElements(dsc.value); + collectionElementsStore.invalidateCollectionElements(dsc.value); + collectionElementsStore.fetchMissingElements(dsc.value, offset.value); }, { deep: true } ); diff --git a/client/src/store/historyStore/model/utilities.js b/client/src/store/historyStore/model/utilities.js index 86b5ac13bc61..6a7ccb2d3f0c 100644 --- a/client/src/store/historyStore/model/utilities.js +++ b/client/src/store/historyStore/model/utilities.js @@ -1,9 +1,9 @@ -import Vue from "vue"; +import { set } from "vue"; /* This function merges the existing data with new incoming data. */ export function mergeArray(id, payload, items, itemKey) { if (!items[id]) { - Vue.set(items, id, []); + set(items, id, []); } const itemArray = items[id]; for (const item of payload) { @@ -16,7 +16,7 @@ export function mergeArray(id, payload, items, itemKey) { }); } } else { - Vue.set(itemArray, itemIndex, item); + set(itemArray, itemIndex, item); } } } diff --git a/client/src/stores/collectionElementsStore.test.ts b/client/src/stores/collectionElementsStore.test.ts index b34435288554..4a2c7be2e2ce 100644 --- a/client/src/stores/collectionElementsStore.test.ts +++ b/client/src/stores/collectionElementsStore.test.ts @@ -38,13 +38,15 @@ describe("useCollectionElementsStore", () => { expect(store.storedCollectionElements).toEqual({}); expect(store.isLoadingCollectionElements(collection)).toEqual(false); - const offset = 0; + // Getting collection elements should be side effect free + store.getCollectionElements(collection); + expect(store.isLoadingCollectionElements(collection)).toEqual(false); + await flushPromises(); + expect(fetchCollectionElements).not.toHaveBeenCalled(); + const limit = 5; - // Getting collection elements should trigger a fetch and change the loading state - store.getCollectionElements(collection, offset, limit); - expect(store.isLoadingCollectionElements(collection)).toEqual(true); + store.fetchMissingElements(collection, 0, limit); await flushPromises(); - expect(store.isLoadingCollectionElements(collection)).toEqual(false); expect(fetchCollectionElements).toHaveBeenCalled(); const collectionKey = store.getCollectionKey(collection); @@ -70,18 +72,20 @@ describe("useCollectionElementsStore", () => { const offset = 0; const limit = storedCount; // Getting the same collection elements range should not trigger a fetch - store.getCollectionElements(collection, offset, limit); + store.fetchMissingElements(collection, offset, limit); expect(store.isLoadingCollectionElements(collection)).toEqual(false); expect(fetchCollectionElements).not.toHaveBeenCalled(); }); it("should fetch only missing elements if the requested range is not already stored", async () => { + jest.useFakeTimers(); + const totalElements = 10; const collection: HDCASummary = mockCollection("1", totalElements); const store = useCollectionElementsStore(); const initialElements = 3; - store.getCollectionElements(collection, 0, initialElements); + store.fetchMissingElements(collection, 0, initialElements); await flushPromises(); expect(fetchCollectionElements).toHaveBeenCalled(); const collectionKey = store.getCollectionKey(collection); @@ -92,11 +96,10 @@ describe("useCollectionElementsStore", () => { const offset = 2; const limit = 5; - // Getting collection elements should trigger a fetch in this case - store.getCollectionElements(collection, offset, limit); - expect(store.isLoadingCollectionElements(collection)).toEqual(true); + // Fetching collection elements should trigger a fetch in this case + store.fetchMissingElements(collection, offset, limit); + jest.runAllTimers(); await flushPromises(); - expect(store.isLoadingCollectionElements(collection)).toEqual(false); expect(fetchCollectionElements).toHaveBeenCalled(); elements = store.storedCollectionElements[collectionKey]; diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts index ddeb60102012..73c63ade28a2 100644 --- a/client/src/stores/collectionElementsStore.ts +++ b/client/src/stores/collectionElementsStore.ts @@ -4,6 +4,8 @@ import { computed, del, ref, set } from "vue"; import type { CollectionEntry, DCESummary, HDCASummary, HistoryContentItemBase } from "@/api"; import { isHDCA } from "@/api"; import { fetchCollectionDetails, fetchElementsFromCollection } from "@/api/datasetCollections"; +import { ensureDefined } from "@/utils/assertions"; +import { ActionSkippedError, LastQueue } from "@/utils/lastQueue"; /** * Represents an element in a collection that has not been fetched yet. @@ -24,7 +26,11 @@ export interface ContentPlaceholder { fetching?: boolean; } -export type DCEEntry = ContentPlaceholder | DCESummary; +export type InvalidDCEEntry = (ContentPlaceholder | DCESummary) & { + valid: false; +}; + +export type DCEEntry = ContentPlaceholder | DCESummary | InvalidDCEEntry; const FETCH_LIMIT = 50; @@ -46,11 +52,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", } const getCollectionElements = computed(() => { - return (collection: CollectionEntry, offset = 0, limit = FETCH_LIMIT) => { - const storedElements = - storedCollectionElements.value[getCollectionKey(collection)] ?? initWithPlaceholderElements(collection); - fetchMissingElements({ collection, storedElements, offset, limit }); - return storedElements; + return (collection: CollectionEntry) => { + return storedCollectionElements.value[getCollectionKey(collection)]; }; }); @@ -60,47 +63,91 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", }; }); - async function fetchMissingElements(params: { - collection: CollectionEntry; + type FetchParams = { storedElements: DCEEntry[]; + collection: CollectionEntry; offset: number; limit: number; - }) { - const collectionKey = getCollectionKey(params.collection); + }; + + async function fetchMissing({ storedElements, collection, offset, limit = FETCH_LIMIT }: FetchParams) { + const collectionKey = getCollectionKey(collection); + try { - // We should fetch only missing (placeholder) elements from the range - const firstMissingIndexInRange = params.storedElements - .slice(params.offset, params.offset + params.limit) - .findIndex((element) => isPlaceholder(element) && !element.fetching); - - if (firstMissingIndexInRange === -1) { - // All elements in the range are already stored or being fetched - return; + if (collection.element_count !== null) { + // We should fetch only missing (placeholder) elements from the range + const firstMissingIndexInRange = storedElements + .slice(offset, offset + limit) + .findIndex((element) => (isPlaceholder(element) && !element.fetching) || isInvalid(element)); + + if (firstMissingIndexInRange === -1) { + // All elements in the range are already stored or being fetched + return; + } + + // Adjust the offset to the first missing element + offset += firstMissingIndexInRange; + } else { + // Edge case where element_count is incorrect + // TODO: remove me once element_count is reported reliably + offset = 0; } - // Adjust the offset to the first missing element - params.offset += firstMissingIndexInRange; set(loadingCollectionElements.value, collectionKey, true); // Mark all elements in the range as fetching - params.storedElements - .slice(params.offset, params.offset + params.limit) + storedElements + .slice(offset, offset + limit) .forEach((element) => isPlaceholder(element) && (element.fetching = true)); + const fetchedElements = await fetchElementsFromCollection({ - entry: params.collection, - offset: params.offset, - limit: params.limit, + entry: collection, + offset: offset, + limit: limit, }); - // Update only the elements that were fetched - params.storedElements.splice(params.offset, fetchedElements.length, ...fetchedElements); - set(storedCollectionElements.value, collectionKey, params.storedElements); + + return { fetchedElements, elementOffset: offset }; } finally { del(loadingCollectionElements.value, collectionKey); } } - async function loadCollectionElements(collection: CollectionEntry) { - const elements = await fetchElementsFromCollection({ entry: collection }); - set(storedCollectionElements.value, getCollectionKey(collection), elements); + const lastQueue = new LastQueue(1000, true); + + async function fetchMissingElements(collection: CollectionEntry, offset: number, limit = FETCH_LIMIT) { + const key = getCollectionKey(collection); + let storedElements = storedCollectionElements.value[key]; + + if (!storedElements) { + storedElements = initWithPlaceholderElements(collection); + set(storedCollectionElements.value, key, storedElements); + } + + try { + const data = await lastQueue.enqueue(fetchMissing, { storedElements, collection, offset, limit }, key); + + if (data) { + const from = data.elementOffset; + const to = from + data.fetchedElements.length; + + for (let index = from; index < to; index++) { + const element = ensureDefined(data.fetchedElements[index - from]); + set(storedElements, index, element); + } + + set(storedCollectionElements.value, key, storedElements); + } + } catch (e) { + if (!(e instanceof ActionSkippedError)) { + throw e; + } + } + } + + async function invalidateCollectionElements(collection: CollectionEntry) { + const storedElements = storedCollectionElements.value[getCollectionKey(collection)] ?? []; + storedElements.forEach((element) => { + (element as InvalidDCEEntry).valid = false; + }); } function saveCollections(historyContentsPayload: HistoryContentItemBase[]) { @@ -135,6 +182,10 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", return "id" in element === false; } + function isInvalid(element: DCEEntry): element is InvalidDCEEntry { + return (element as InvalidDCEEntry)["valid"] === false; + } + function initWithPlaceholderElements(collection: CollectionEntry): ContentPlaceholder[] { const totalElements = collection.element_count ?? 0; const placeholderElements = new Array(totalElements); @@ -151,8 +202,9 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", isLoadingCollectionElements, getCollection, fetchCollection, - loadCollectionElements, + invalidateCollectionElements, saveCollections, getCollectionKey, + fetchMissingElements, }; }); diff --git a/client/src/utils/lastQueue.ts b/client/src/utils/lastQueue.ts index 317d7dfa3a3c..ff27690f0b5f 100644 --- a/client/src/utils/lastQueue.ts +++ b/client/src/utils/lastQueue.ts @@ -1,4 +1,4 @@ -type QueuedAction R, R = unknown> = { +type QueuedAction R, R = ReturnType> = { action: T; arg: Parameters[0]; resolve: (value: R) => void; @@ -13,7 +13,7 @@ export class ActionSkippedError extends Error {} * This is useful when promises earlier enqueued become obsolete. * See also: https://stackoverflow.com/questions/53540348/js-async-await-tasks-queue */ -export class LastQueue R, R = unknown> { +export class LastQueue R, R = ReturnType> { throttlePeriod: number; /** Throw an error if a queued action is skipped. This avoids dangling promises */ rejectSkipped: boolean; @@ -34,7 +34,7 @@ export class LastQueue R, R = unknown> { promise?.reject(new ActionSkippedError()); } - async enqueue(action: T, arg: Parameters[0], key: string | number = 0) { + async enqueue(action: T, arg: Parameters[0], key: string | number = 0): Promise { return new Promise((resolve, reject) => { this.skipPromise(key); this.queuedPromises[key] = { action, arg, resolve, reject };