Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.1] Fix infinite rapid polling in useKeyedCache #18756

Merged
merged 1 commit into from
Sep 2, 2024
Merged

[24.1] Fix infinite rapid polling in useKeyedCache #18756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions client/src/components/Collections/common/CollectionEditView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,26 @@ const collectionChangeKey = ref(0);
const attributesData = computed(() => {
return collectionAttributesStore.getAttributes(props.collectionId);
});
const attributesLoadError = computed(() =>
errorMessageAsString(collectionAttributesStore.hasItemLoadError(props.collectionId))
);

const collection = computed(() => {
return collectionStore.getCollectionById(props.collectionId);
});
const collectionLoadError = computed(() => {
if (collection.value) {
return errorMessageAsString(collectionStore.hasLoadingCollectionElementsError(collection.value));
}
return "";
});
watch([attributesLoadError, collectionLoadError], () => {
if (attributesLoadError.value) {
errorMessage.value = attributesLoadError.value;
} else if (collectionLoadError.value) {
errorMessage.value = collectionLoadError.value;
}
});
const databaseKeyFromElements = computed(() => {
return attributesData.value?.dbkey;
});
Expand Down Expand Up @@ -101,7 +118,7 @@ async function clickedSave(attribute: string, newValue: any) {
try {
await copyCollection(props.collectionId, dbKey);
} catch (err) {
errorMessage.value = errorMessageAsString(err, "History import failed.");
errorMessage.value = errorMessageAsString(err, `Changing ${attribute} failed`);
}
}

Expand All @@ -119,7 +136,7 @@ async function clickedConvert(selectedConverter: any) {
await axios.post(url, data).catch(handleError);
successMessage.value = "Conversion started successfully.";
} catch (err) {
errorMessage.value = errorMessageAsString(err, "History import failed.");
errorMessage.value = errorMessageAsString(err, "Conversion failed.");
}
}

Expand All @@ -143,12 +160,12 @@ async function clickedDatatypeChange(selectedDatatype: any) {
await updateHistoryItemsBulk(currentHistoryId.value ?? "", data);
successMessage.value = "Datatype changed successfully.";
} catch (err) {
errorMessage.value = errorMessageAsString(err, "History import failed.");
errorMessage.value = errorMessageAsString(err, "Datatype change failed.");
}
}

function handleError(err: any) {
errorMessage.value = errorMessageAsString(err, "History import failed.");
errorMessage.value = errorMessageAsString(err, "Datatype conversion failed.");

if (err?.data?.stderr) {
jobError.value = err.data;
Expand Down Expand Up @@ -191,14 +208,14 @@ async function saveAttrs() {
{{ localize(infoMessage) }}
</BAlert>

<BAlert v-if="jobError" show variant="danger" dismissible>
<BAlert v-if="errorMessage" show variant="danger">
{{ localize(errorMessage) }}
</BAlert>

<BAlert v-if="successMessage" show variant="success" dismissible>
{{ localize(successMessage) }}
</BAlert>
<BTabs class="mt-3">
<BTabs v-if="!errorMessage" class="mt-3">
<BTab title-link-class="collection-edit-attributes-nav" @click="updateInfoMessage('')">
<template v-slot:title>
<FontAwesomeIcon :icon="faBars" class="mr-1" />
Expand Down
8 changes: 7 additions & 1 deletion client/src/components/History/CurrentCollection/CollectionPanel.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import { canMutateHistory, isCollectionElement, isHDCA } from "@/api";
import ExpandedItems from "@/components/History/Content/ExpandedItems";
import { updateContentFields } from "@/components/History/model/queries";
import { useCollectionElementsStore } from "@/stores/collectionElementsStore";
import { errorMessageAsString } from "@/utils/simple-error";

import CollectionDetails from "./CollectionDetails.vue";
import CollectionNavigation from "./CollectionNavigation.vue";
import CollectionOperations from "./CollectionOperations.vue";
import Alert from "@/components/Alert.vue";
import ContentItem from "@/components/History/Content/ContentItem.vue";
import ListingLayout from "@/components/History/Layout/ListingLayout.vue";

Expand Down Expand Up @@ -54,6 +56,7 @@ watch(

const collectionElements = computed(() => collectionElementsStore.getCollectionElements(dsc.value) ?? []);
const loading = computed(() => collectionElementsStore.isLoadingCollectionElements(dsc.value));
const error = computed(() => collectionElementsStore.hasLoadingCollectionElementsError(dsc.value));
const jobState = computed(() => ("job_state_summary" in dsc.value ? dsc.value.job_state_summary : undefined));
const populatedStateMsg = computed(() =>
"populated_state_message" in dsc.value ? dsc.value.populated_state_message : undefined
Expand Down Expand Up @@ -118,7 +121,10 @@ watch(
</script>

<template>
<ExpandedItems v-slot="{ isExpanded, setExpanded }" :scope-key="dsc.id" :get-item-key="getItemKey">
<Alert v-if="error" variant="error">
{{ errorMessageAsString(error) }}
</Alert>
<ExpandedItems v-else v-slot="{ isExpanded, setExpanded }" :scope-key="dsc.id" :get-item-key="getItemKey">
<section class="dataset-collection-panel w-100 d-flex flex-column">
<section>
<CollectionNavigation
Expand Down
16 changes: 15 additions & 1 deletion client/src/composables/keyedCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function useKeyedCache<T>(
) {
const storedItems = ref<{ [key: string]: T }>({});
const loadingItem = ref<{ [key: string]: boolean }>({});
const loadingErrors = ref<{ [key: string]: Error }>({});

const getItemById = computed(() => {
return (id: string) => {
Expand All @@ -69,10 +70,17 @@ export function useKeyedCache<T>(
};
});

const hasItemLoadError = computed(() => {
return (id: string) => {
return loadingErrors.value[id] ?? null;
};
});

async function fetchItemById(params: FetchParams) {
const itemId = params.id;
const isAlreadyLoading = loadingItem.value[itemId] ?? false;
if (isAlreadyLoading) {
const failedLoading = loadingErrors.value[itemId];
if (isAlreadyLoading || failedLoading) {
return;
}
set(loadingItem.value, itemId, true);
Expand All @@ -81,6 +89,8 @@ export function useKeyedCache<T>(
const { data } = await fetchItem({ id: itemId });
set(storedItems.value, itemId, data);
return data;
} catch (error) {
set(loadingErrors.value, itemId, error);
} finally {
del(loadingItem.value, itemId);
}
Expand All @@ -100,6 +110,10 @@ export function useKeyedCache<T>(
/**
* A computed function that returns true if the item with the given id is currently being fetched.
*/
hasItemLoadError,
/**
* A computed function holding errors
*/
isLoadingItem,
/**
* Fetches the item with the given id from the server.
Expand Down
5 changes: 3 additions & 2 deletions client/src/stores/collectionAttributesStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import { fetchCollectionAttributes } from "@/api/datasetCollections";
import { useKeyedCache } from "@/composables/keyedCache";

export const useCollectionAttributesStore = defineStore("collectionAttributesStore", () => {
const { storedItems, getItemById, isLoadingItem } = useKeyedCache<DatasetCollectionAttributes>((params) =>
fetchCollectionAttributes({ id: params.id, instance_type: "history" })
const { storedItems, getItemById, isLoadingItem, hasItemLoadError } = useKeyedCache<DatasetCollectionAttributes>(
(params) => fetchCollectionAttributes({ id: params.id, instance_type: "history" })
);

return {
storedAttributes: storedItems,
getAttributes: getItemById,
isLoadingAttributes: isLoadingItem,
hasItemLoadError: hasItemLoadError,
};
});
15 changes: 14 additions & 1 deletion client/src/stores/collectionElementsStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const FETCH_LIMIT = 50;
export const useCollectionElementsStore = defineStore("collectionElementsStore", () => {
const storedCollections = ref<{ [key: string]: HDCASummary }>({});
const loadingCollectionElements = ref<{ [key: string]: boolean }>({});
const loadingCollectionElementsErrors = ref<{ [key: string]: Error }>({});
const storedCollectionElements = ref<{ [key: string]: DCEEntry[] }>({});

/**
Expand All @@ -63,6 +64,12 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
};
});

const hasLoadingCollectionElementsError = computed(() => {
return (collection: CollectionEntry) => {
return loadingCollectionElementsErrors.value[getCollectionKey(collection) ?? false];
};
});

type FetchParams = {
storedElements: DCEEntry[];
collection: CollectionEntry;
Expand Down Expand Up @@ -106,6 +113,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
});

return { fetchedElements, elementOffset: offset };
} catch (error) {
set(loadingCollectionElementsErrors.value, collectionKey, error);
} finally {
del(loadingCollectionElements.value, collectionKey);
}
Expand Down Expand Up @@ -162,7 +171,7 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
/** Returns collection from storedCollections, will load collection if not in store */
const getCollectionById = computed(() => {
return (collectionId: string) => {
if (!storedCollections.value[collectionId]) {
if (!storedCollections.value[collectionId] && !loadingCollectionElementsErrors.value[collectionId]) {
// TODO: Try to remove this as it can cause computed side effects (use keyedCache in this store instead?)
fetchCollection({ id: collectionId });
}
Expand All @@ -176,6 +185,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
const collection = await fetchCollectionDetails({ id: params.id });
set(storedCollections.value, collection.id, collection);
return collection;
} catch (error) {
set(loadingCollectionElementsErrors.value, params.id, error);
} finally {
del(loadingCollectionElements.value, params.id);
}
Expand Down Expand Up @@ -203,6 +214,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
storedCollectionElements,
getCollectionElements,
isLoadingCollectionElements,
hasLoadingCollectionElementsError,
loadingCollectionElementsErrors,
getCollectionById,
fetchCollection,
invalidateCollectionElements,
Expand Down
Loading