From e40c17675f8efb0f9c5f05b0b5b966da9fd82267 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Mon, 8 May 2023 11:12:55 -0400 Subject: [PATCH] Export: fix conflicts and timing (#2121) --- Backend/Controllers/LiftController.cs | 3 +- Backend/Services/LiftService.cs | 2 +- src/backend/index.ts | 10 +- src/components/App/SignalRHub.tsx | 105 +++++++++++------- .../ProjectExport/DownloadButton.tsx | 21 ++-- src/components/ProjectExport/ExportButton.tsx | 6 +- .../Redux/ExportProjectActions.ts | 14 +-- 7 files changed, 92 insertions(+), 69 deletions(-) diff --git a/Backend/Controllers/LiftController.cs b/Backend/Controllers/LiftController.cs index ad11455951..22216ef2a9 100644 --- a/Backend/Controllers/LiftController.cs +++ b/Backend/Controllers/LiftController.cs @@ -257,8 +257,9 @@ internal async Task CreateLiftExportThenSignal(string projectId, string us } catch { - await _notifyService.Clients.All.SendAsync(CombineHub.ExportFailed, userId); _logger.LogError("Error exporting project {ProjectId}", projectId); + _liftService.DeleteExport(userId); + await _notifyService.Clients.All.SendAsync(CombineHub.ExportFailed, userId); throw; } } diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index e4a91c3a8b..9b1b52d842 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -144,7 +144,7 @@ public void StoreExport(string userId, string filePath) public bool DeleteExport(string userId) { var removeSuccessful = _liftExports.Remove(userId, out var filePath); - if (removeSuccessful && filePath is not null) + if (removeSuccessful && filePath is not null && filePath != InProgress) { File.Delete(filePath); } diff --git a/src/backend/index.ts b/src/backend/index.ts index bf2ae43741..571136dbde 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -252,10 +252,12 @@ export async function downloadLift(projectId: string): Promise { ); } -/** After downloading a LIFT file, clear it from the backend. */ -export async function deleteLift(projectId?: string): Promise { - projectId = projectId ? projectId : LocalStorage.getProjectId(); - await liftApi.deleteLiftFile({ projectId }, defaultOptions()); +/** After downloading a LIFT file, clear it from the backend. + * The backend deletes by user, not by project, + * but a nonempty projectId in the url is still required. + */ +export async function deleteLift(): Promise { + await liftApi.deleteLiftFile({ projectId: "nonempty" }, defaultOptions()); } export async function canUploadLift(): Promise { diff --git a/src/components/App/SignalRHub.tsx b/src/components/App/SignalRHub.tsx index 3c97e31d12..7b829f8c5d 100644 --- a/src/components/App/SignalRHub.tsx +++ b/src/components/App/SignalRHub.tsx @@ -1,11 +1,15 @@ -import { HubConnection, HubConnectionBuilder } from "@microsoft/signalr"; -import React, { useEffect, useState } from "react"; +import { + HubConnection, + HubConnectionBuilder, + HubConnectionState, +} from "@microsoft/signalr"; +import { Fragment, useCallback, useEffect, useState } from "react"; import { baseURL } from "backend"; import { getUserId } from "backend/localStorage"; import { - downloadIsReady, failure, + success, } from "components/ProjectExport/Redux/ExportProjectActions"; import { ExportStatus } from "components/ProjectExport/Redux/ExportProjectReduxTypes"; import { StoreState } from "types"; @@ -18,57 +22,74 @@ export default function SignalRHub() { ); const dispatch = useAppDispatch(); const [connection, setConnection] = useState(); + const [disconnect, setDisconnect] = useState(false); + const [reconnect, setReconnect] = useState(false); + + const finishDisconnect = useCallback((): void => { + setConnection(undefined); + setDisconnect(false); + }, [setConnection, setDisconnect]); + /** Act on the disconnect state to stop and delete the connection. */ useEffect(() => { - if (connection) { - connection.stop(); + if (disconnect) { + if (connection) { + connection.stop().then(finishDisconnect).catch(finishDisconnect); + } else { + setDisconnect(false); + } } - setConnection(undefined); - if (exportState.status === ExportStatus.Exporting) { + }, [connection, disconnect, finishDisconnect]); + + /** Once disconnect state is acted on, act on the reconnect state. */ + useEffect(() => { + if (!disconnect && reconnect) { const newConnection = new HubConnectionBuilder() .withUrl(`${baseURL}/hub`) .withAutomaticReconnect() .build(); + setReconnect(false); setConnection(newConnection); } - // We reference connection, but don't want it in the dependency list. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [exportState]); + }, [disconnect, reconnect, setConnection, setReconnect]); + /** Any change in exportState should cause a disconnect. + * Only ExportStatus.Exporting should open a new connection. + */ useEffect(() => { - if (connection) { - // Name must match what is in Backend/Helper/CombineHub.cs. - const failName = "ExportFailed"; - const successName = "DownloadReady"; - - // The method is what the frontend does upon message receipt. - const failMethod = (userId: string) => { - if (userId === getUserId()) { - dispatch(failure(exportState.projectId)); - // After dispatch, stop the connection completely. - // We don't need it active unless a new export is started, - // and that might be with a different projectId. - connection.stop(); - } - }; - const successMethod = (userId: string) => { - if (userId === getUserId()) { - dispatch(downloadIsReady(exportState.projectId)); - // After dispatch, stop the connection completely. - // We don't need it active unless a new export is started, - // and that might be with a different projectId. - connection.stop(); - } - }; + setDisconnect(true); + if (exportState.status === ExportStatus.Exporting) { + setReconnect(true); + } + }, [exportState, setDisconnect, setReconnect]); - connection.start().then(() => { - connection.on(failName, failMethod); - connection.on(successName, successMethod); - }); + /** Once a connection is opened, start the relevant methods. */ + useEffect(() => { + if (connection?.state !== HubConnectionState.Disconnected) { + return; } - // We reference dispatch and exportState, but they're not dependencies. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [connection]); - return ; + // Name must match what is in Backend/Helper/CombineHub.cs. + const failName = "ExportFailed"; + const successName = "DownloadReady"; + + // The method is what the frontend does upon message receipt. + const failMethod = (userId: string): void => { + if (userId === getUserId()) { + dispatch(failure(exportState.projectId)); + } + }; + const successMethod = (userId: string): void => { + if (userId === getUserId()) { + dispatch(success(exportState.projectId)); + } + }; + + connection.start().then(() => { + connection.on(failName, failMethod); + connection.on(successName, successMethod); + }); + }, [connection, dispatch, exportState.projectId]); + + return ; } diff --git a/src/components/ProjectExport/DownloadButton.tsx b/src/components/ProjectExport/DownloadButton.tsx index 81213cd40d..0df123ced2 100644 --- a/src/components/ProjectExport/DownloadButton.tsx +++ b/src/components/ProjectExport/DownloadButton.tsx @@ -1,7 +1,8 @@ import { Cached, Error as ErrorIcon } from "@mui/icons-material"; import { IconButton, Tooltip } from "@mui/material"; -import React, { +import { createRef, + Fragment, ReactElement, useCallback, useEffect, @@ -12,7 +13,7 @@ import { useTranslation } from "react-i18next"; import { getProjectName } from "backend"; import { asyncDownloadExport, - resetExport, + asyncResetExport, } from "components/ProjectExport/Redux/ExportProjectActions"; import { ExportStatus } from "components/ProjectExport/Redux/ExportProjectReduxTypes"; import { StoreState } from "types"; @@ -32,7 +33,9 @@ interface DownloadButtonProps { * A button to show export status. This automatically initiates a download * when a user's export is done, so there should be exactly one copy of this * component rendered at any given time in the logged-in app. */ -export default function DownloadButton(props: DownloadButtonProps) { +export default function DownloadButton( + props: DownloadButtonProps +): ReactElement { const exportState = useAppSelector( (state: StoreState) => state.exportProjectState ); @@ -42,9 +45,9 @@ export default function DownloadButton(props: DownloadButtonProps) { const { t } = useTranslation(); const downloadLink = createRef(); - const reset = useCallback(() => { - dispatch(resetExport(exportState.projectId)); - }, [dispatch, exportState.projectId]); + const reset = useCallback(async (): Promise => { + await dispatch(asyncResetExport()); + }, [dispatch]); useEffect(() => { if (downloadLink.current && fileUrl) { @@ -97,7 +100,7 @@ export default function DownloadButton(props: DownloadButtonProps) { case ExportStatus.Failure: return ; default: - return
; + return ; } } @@ -119,7 +122,7 @@ export default function DownloadButton(props: DownloadButtonProps) { } return ( - + {exportState.status !== ExportStatus.Default && ( )} - + ); } diff --git a/src/components/ProjectExport/ExportButton.tsx b/src/components/ProjectExport/ExportButton.tsx index 1c1d5d7074..6724f3e2f6 100644 --- a/src/components/ProjectExport/ExportButton.tsx +++ b/src/components/ProjectExport/ExportButton.tsx @@ -20,10 +20,10 @@ export default function ExportButton(props: ExportButtonProps) { const { t } = useTranslation(); const { enqueueSnackbar } = useSnackbar(); - function exportProj() { - isFrontierNonempty(props.projectId).then((isNonempty) => { + async function exportProj() { + await isFrontierNonempty(props.projectId).then(async (isNonempty) => { if (isNonempty) { - dispatch(asyncExportProject(props.projectId)); + await dispatch(asyncExportProject(props.projectId)); } else { enqueueSnackbar(t("projectExport.cannotExportEmpty")); } diff --git a/src/components/ProjectExport/Redux/ExportProjectActions.ts b/src/components/ProjectExport/Redux/ExportProjectActions.ts index 3aff89da67..cfea4ba26a 100644 --- a/src/components/ProjectExport/Redux/ExportProjectActions.ts +++ b/src/components/ProjectExport/Redux/ExportProjectActions.ts @@ -8,14 +8,10 @@ import { StoreStateDispatch } from "types/Redux/actions"; export function asyncExportProject(projectId: string) { return async (dispatch: StoreStateDispatch) => { dispatch(exporting(projectId)); - exportLift(projectId).catch(() => dispatch(failure(projectId))); + await exportLift(projectId).catch(() => dispatch(failure(projectId))); }; } -export function downloadIsReady(projectId: string) { - return (dispatch: StoreStateDispatch) => dispatch(success(projectId)); -} - export function asyncDownloadExport(projectId: string) { return async (dispatch: StoreStateDispatch) => { dispatch(downloading(projectId)); @@ -25,10 +21,10 @@ export function asyncDownloadExport(projectId: string) { }; } -export function resetExport(projectId?: string) { - return (dispatch: StoreStateDispatch) => { +export function asyncResetExport() { + return async (dispatch: StoreStateDispatch) => { dispatch(reset()); - deleteLift(projectId); + await deleteLift(); }; } @@ -44,7 +40,7 @@ function downloading(projectId: string): ExportProjectAction { projectId, }; } -function success(projectId: string): ExportProjectAction { +export function success(projectId: string): ExportProjectAction { return { type: ExportStatus.Success, projectId,