Skip to content

Commit

Permalink
Eda study data race condition (#1132)
Browse files Browse the repository at this point in the history
* Add options to usePromise hook

* Use new options of usePromise hook, to prevent inconsistent data

* Return the `value` property of `usePromise` result

(cherry picked from commit f2a73f9)
  • Loading branch information
dmfalke committed Jul 11, 2024
1 parent ac9f56f commit 501640f
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function EDAAnalysisListContainer(props: Props) {
const studyMetadata = useStudyMetadata(studyId, subsettingClient);
const contextValue = useDeepValue({
...studyRecordState,
studyMetadata,
studyMetadata: studyMetadata.value,
analysisClient,
subsettingClient,
dataClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ export function EDAWorkspaceContainer(props: Props) {

const wdkStudyRecordState = useWdkStudyRecord(studyId);
const studyMetadata = useStudyMetadata(studyId, subsettingClient);
if (wdkStudyRecordState == null || studyMetadata == null) return <Loading />;
if (wdkStudyRecordState == null || studyMetadata.value == null)
return <Loading />;
return (
<EDAWorkspaceContainerWithLoadedData
{...props}
wdkStudyRecord={wdkStudyRecordState}
studyMetadata={studyMetadata}
studyMetadata={studyMetadata.value}
/>
);
}
Expand Down
18 changes: 15 additions & 3 deletions packages/libs/eda/src/lib/core/hooks/promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ export type PromiseHookState<T> = {
error?: unknown;
};

export type PromiseHookOptions = {
keepPreviousValue?: boolean;
throwError?: boolean;
};

/**
* Invokes `task` and returns an object representing its current state and resolved value.
* The last resolved value will remain util a new promise is resolved.
Expand All @@ -15,15 +20,19 @@ export type PromiseHookState<T> = {
* @param task A function that returns a promise
* @returns PromiseHookState<T>
*/
export function usePromise<T>(task: () => Promise<T>): PromiseHookState<T> {
export function usePromise<T>(
task: () => Promise<T>,
options: PromiseHookOptions = {}
): PromiseHookState<T> {
const { keepPreviousValue = true, throwError = false } = options;
const [state, setState] = useState<PromiseHookState<T>>({
pending: true,
});
useEffect(() => {
let ignoreResolve = false;
setState((prev) => ({
pending: true,
value: prev.value,
value: keepPreviousValue ? prev.value : undefined,
error: undefined,
}));
task().then(
Expand All @@ -36,6 +45,9 @@ export function usePromise<T>(task: () => Promise<T>): PromiseHookState<T> {
},
(error) => {
if (ignoreResolve) return;
if (throwError) {
throw error;
}
setState({
error,
pending: false,
Expand All @@ -45,6 +57,6 @@ export function usePromise<T>(task: () => Promise<T>): PromiseHookState<T> {
return function cleanup() {
ignoreResolve = true;
};
}, [task]);
}, [keepPreviousValue, task, throwError]);
return state;
}
10 changes: 6 additions & 4 deletions packages/libs/eda/src/lib/core/hooks/study.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export function isStubEntity(entity: StudyEntity) {

export function useStudyMetadata(datasetId: string, client: SubsettingClient) {
const permissionsResponse = usePermissions();
const { error, value } = usePromise(
return usePromise(
useCallback(async () => {
if (permissionsResponse.loading) return;
const { permissions } = permissionsResponse;
Expand All @@ -208,8 +208,10 @@ export function useStudyMetadata(datasetId: string, client: SubsettingClient) {
}
throw error;
}
}, [client, datasetId, permissionsResponse])
}, [client, datasetId, permissionsResponse]),
{
keepPreviousValue: false,
throwError: true,
}
);
if (error) throw error;
return value;
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const userDatasetRoutes: RouteEntry[] = [
function useEdaStudyMetadata(wdkDatasetId: string) {
try {
const subsettingClient = useConfiguredSubsettingClient(edaServiceUrl);
return useStudyMetadata(wdkDatasetId, subsettingClient);
return useStudyMetadata(wdkDatasetId, subsettingClient).value;
} catch (error) {
console.error(error);
return undefined;
Expand Down

0 comments on commit 501640f

Please sign in to comment.