Export: fix conflicts and timing (#2121)

Backend/Controllers/LiftController.cs

@@ -257,8 +257,9 @@ internal async Task<bool> 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;
             }
         }

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);
             }

src/backend/index.ts

@@ -252,10 +252,12 @@ export async function downloadLift(projectId: string): Promise<string> {
   );
 }
 
-/** After downloading a LIFT file, clear it from the backend. */
-export async function deleteLift(projectId?: string): Promise<void> {
-  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<void> {
+  await liftApi.deleteLiftFile({ projectId: "nonempty" }, defaultOptions());
 }
 
 export async function canUploadLift(): Promise<boolean> {

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<HubConnection | undefined>();
+  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 <React.Fragment />;
+    // 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 <Fragment />;
 }

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<HTMLAnchorElement>();
 
-  const reset = useCallback(() => {
-    dispatch(resetExport(exportState.projectId));
-  }, [dispatch, exportState.projectId]);
+  const reset = useCallback(async (): Promise<void> => {
+    await dispatch(asyncResetExport());
+  }, [dispatch]);
 
   useEffect(() => {
     if (downloadLink.current && fileUrl) {
@@ -97,7 +100,7 @@ export default function DownloadButton(props: DownloadButtonProps) {
       case ExportStatus.Failure:
         return <ErrorIcon />;
       default:
-        return <div />;
+        return <Fragment />;
     }
   }
 
@@ -119,7 +122,7 @@ export default function DownloadButton(props: DownloadButtonProps) {
   }
 
   return (
-    <React.Fragment>
+    <Fragment>
       {exportState.status !== ExportStatus.Default && (
         <Tooltip title={t(textId())} placement="bottom">
           <IconButton
@@ -142,6 +145,6 @@ export default function DownloadButton(props: DownloadButtonProps) {
           (This link should not be visible)
         </a>
       )}
-    </React.Fragment>
+    </Fragment>
   );
 }

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"));
       }

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,