Merge pull request natcap#1279 from davemfish/bugfix/WB-1070-download…

…-permissions

Error-handling for sampledata downloading in workbench

HISTORY.rst

@@ -37,7 +37,10 @@
 
 Unreleased Changes
 ------------------
-
+* Workbench
+    * Fixed a bug where sampledata downloads failed silently (and progress bar 
+      became innacurate) if the Workbench did not have write permission to 
+      the download location. https://github.com/natcap/invest/issues/1070
 * HRA
     * Fixed a bug in HRA where the model would error when all exposure and
       consequence criteria were skipped for a single habitat. The model now

workbench/src/main/ipcMainChannels.js

@@ -1,4 +1,5 @@
 export const ipcMainChannels = {
+  CHECK_FILE_PERMISSIONS: 'check-file-permissions',
   CHECK_STORAGE_TOKEN: 'check-storage-token',
   DOWNLOAD_URL: 'download-url',
   GET_N_CPUS: 'get-n-cpus',

workbench/src/main/main.js

@@ -19,6 +19,7 @@ import findInvestBinaries from './findInvestBinaries';
 import setupDownloadHandlers from './setupDownloadHandlers';
 import setupDialogs from './setupDialogs';
 import setupContextMenu from './setupContextMenu';
+import setupCheckFilePermissions from './setupCheckFilePermissions';
 import { setupCheckFirstRun } from './setupCheckFirstRun';
 import { setupCheckStorageToken } from './setupCheckStorageToken';
 import {
@@ -139,6 +140,7 @@ export const createWindow = async () => {
     mainWindow = null;
   });
 
+  setupCheckFilePermissions();
   setupDownloadHandlers(mainWindow);
   setupInvestRunHandlers(investExe);
   setupInvestLogReaderHandler();

workbench/src/main/setupCheckFilePermissions.js

@@ -0,0 +1,32 @@
+import fs from 'fs';
+import path from 'path';
+
+import {
+  ipcMain,
+} from 'electron';
+
+import { ipcMainChannels } from './ipcMainChannels';
+import { getLogger } from './logger';
+
+const logger = getLogger(__filename.split('/').slice(-1)[0]);
+
+export default function setupCheckFilePermissions() {
+  ipcMain.handle(
+    ipcMainChannels.CHECK_FILE_PERMISSIONS, (event, folder) => {
+      const filepath = path.join(folder, 'foo.txt');
+      let writeable;
+      try {
+        // The only reliable way to determine if a folder is writeable
+        // is to write to it. https://github.com/nodejs/node/issues/2949
+        fs.writeFileSync(filepath, '');
+        writeable = true;
+      } catch (err) {
+        writeable = false;
+        logger.debug(err);
+      } finally {
+        fs.rm(filepath, (err) => logger.debug(err));
+      }
+      return writeable;
+    }
+  );
+}

workbench/src/main/setupDownloadHandlers.js

@@ -45,6 +45,7 @@ export default function setupDownloadHandlers(mainWindow) {
     const itemURL = item.getURL();
     item.on('updated', (event, state) => {
       if (state === 'interrupted') {
+        item.cancel(); // we're never attempting to resume, so cancel.
         logger.info('download interrupted');
       } else if (state === 'progressing') {
         if (item.isPaused()) {
@@ -75,6 +76,8 @@ export default function setupDownloadHandlers(mainWindow) {
         );
       } else {
         logger.info(`download failed: ${state}`);
+        const idx = downloadQueue.findIndex((item) => item === itemURL);
+        downloadQueue.splice(idx, 1);
         mainWindow.webContents.send(
           'download-status',
           ['failed', 'failed'] // ProgressBar expects array length 2

workbench/src/renderer/components/DataDownloadModal/index.jsx

@@ -5,18 +5,21 @@ import Form from 'react-bootstrap/Form';
 import Button from 'react-bootstrap/Button';
 import Modal from 'react-bootstrap/Modal';
 import Alert from 'react-bootstrap/Alert';
-import ProgressBar from 'react-bootstrap/ProgressBar';
 import Table from 'react-bootstrap/Table';
+import {
+  MdErrorOutline,
+} from 'react-icons/md';
 import { withTranslation } from 'react-i18next';
 
-import Expire from '../Expire';
 import sampledataRegistry from './sampledata_registry.json';
 import { ipcMainChannels } from '../../../main/ipcMainChannels';
 
 const { ipcRenderer } = window.Workbench.electron;
 const logger = window.Workbench.getLogger('DataDownloadModal');
 
-const BASE_URL = 'https://storage.googleapis.com/releases.naturalcapitalproject.org/invest/3.10.2/data';
+// A URL for sampledata to use in devMode, when the token containing the URL
+// associated with a production build of the Workbench does not exist.
+const BASE_URL = 'https://storage.googleapis.com/releases.naturalcapitalproject.org/invest/3.13.0/data';
 const DEFAULT_FILESIZE = 0;
 
 /** Render a dialog with a form for configuring global invest settings */
@@ -30,11 +33,13 @@ class DataDownloadModal extends React.Component {
       modelCheckBoxState: {},
       dataRegistry: null,
       baseURL: BASE_URL,
+      alertPath: '',
     };
 
     this.handleSubmit = this.handleSubmit.bind(this);
     this.handleCheckAll = this.handleCheckAll.bind(this);
     this.handleCheckList = this.handleCheckList.bind(this);
+    this.closeDialog = this.closeDialog.bind(this);
     this.controller = new AbortController();
     this.signal = this.controller.signal;
   }
@@ -92,14 +97,22 @@ class DataDownloadModal extends React.Component {
       { properties: ['openDirectory'] }
     );
     if (data.filePaths.length) {
-      ipcRenderer.send(
-        ipcMainChannels.DOWNLOAD_URL,
-        this.state.selectedLinksArray,
-        data.filePaths[0]
+      const writable = await ipcRenderer.invoke(
+        ipcMainChannels.CHECK_FILE_PERMISSIONS, data.filePaths[0]
       );
-      this.props.storeDownloadDir(data.filePaths[0]);
+      if (!writable) {
+        this.setState({ alertPath: data.filePaths[0] });
+      } else {
+        this.setState({ alertPath: '' });
+        ipcRenderer.send(
+          ipcMainChannels.DOWNLOAD_URL,
+          this.state.selectedLinksArray,
+          data.filePaths[0]
+        );
+        this.props.storeDownloadDir(data.filePaths[0]);
+        this.closeDialog();
+      }
     }
-    this.props.closeModal();
   }
 
   handleCheckAll(event) {
@@ -147,6 +160,11 @@ class DataDownloadModal extends React.Component {
     });
   }
 
+  closeDialog() {
+    this.setState({ alertPath: '' })
+    this.props.closeModal()
+  }
+
   render() {
     const {
       modelCheckBoxState,
@@ -192,12 +210,29 @@ class DataDownloadModal extends React.Component {
     return (
       <Modal className="download-data-modal"
         show={this.props.show}
-        onHide={this.props.closeModal}
+        onHide={this.closeDialog}
         size="lg"
       >
         <Form>
           <Modal.Header>
-            <Modal.Title>{t("Download InVEST sample data")}</Modal.Title>
+            {
+              (this.state.alertPath)
+                ? (
+                  <Alert
+                    className="mb-0"
+                    variant="danger"
+                  >
+                    <MdErrorOutline
+                      size="2em"
+                      className="pr-1"
+                    />
+                    {t('Please choose a different folder. '
+                      + 'This application does not have permission to write to folder:')}
+                    <p className="mb-0"><em>{this.state.alertPath}</em></p>
+                  </Alert>
+                )
+                : <Modal.Title>{t("Download InVEST sample data")}</Modal.Title>
+            }
           </Modal.Header>
           <Modal.Body>
             <Table
@@ -227,7 +262,7 @@ class DataDownloadModal extends React.Component {
           <Modal.Footer>
             <Button
               variant="secondary"
-              onClick={this.props.closeModal}
+              onClick={this.closeDialog}
             >
               {t("Cancel")}
             </Button>

workbench/tests/main/main.test.js

@@ -187,6 +187,7 @@ describe('createWindow', () => {
       ipcMainChannels.GET_N_CPUS,
       ipcMainChannels.INVEST_VERSION,
       ipcMainChannels.CHECK_STORAGE_TOKEN,
+      ipcMainChannels.CHECK_FILE_PERMISSIONS,
     ];
     const expectedOnChannels = [
       ipcMainChannels.DOWNLOAD_URL,

workbench/tests/renderer/downloadmodal.test.js

@@ -185,6 +185,9 @@ describe('Integration tests with main process', () => {
       if (channel === ipcMainChannels.SHOW_OPEN_DIALOG) {
         return Promise.resolve(dialogData);
       }
+      if (channel === ipcMainChannels.CHECK_FILE_PERMISSIONS) {
+        return Promise.resolve(true);
+      }
       return Promise.resolve(undefined);
     });
 
@@ -203,8 +206,8 @@ describe('Integration tests with main process', () => {
     });
     const progressBar = await findByRole('progressbar');
     expect(progressBar).toHaveTextContent(`Downloading 1 of ${nURLs}`);
-    // We don't have mocks that take us all the way through to a complete
-    // download, when the progress bar would become a 'Download Complete' alert
+    // The electron window's downloadURL function is mocked, so we don't
+    // expect the progress bar to update further in this test.
   });
 
   test('Cancel: does not store a sampleDataDir value', async () => {
@@ -219,4 +222,29 @@ describe('Integration tests with main process', () => {
       expect(value).toBe(existingValue);
     });
   });
+
+  test('Alert when download location is not writeable', async () => {
+    const dialogData = {
+      filePaths: ['foo/directory'],
+    };
+
+    ipcRenderer.invoke.mockImplementation((channel, options) => {
+      if (channel === ipcMainChannels.SHOW_OPEN_DIALOG) {
+        return Promise.resolve(dialogData);
+      }
+      if (channel === ipcMainChannels.CHECK_FILE_PERMISSIONS) {
+        return Promise.resolve(false);
+      }
+      return Promise.resolve(undefined);
+    });
+
+    const {
+      findByRole,
+    } = render(<App isFirstRun />);
+
+    const downloadButton = await findByRole('button', { name: 'Download' });
+    userEvent.click(downloadButton);
+    const alert = await findByRole('alert');
+    expect(alert).toHaveTextContent('Please choose a different folder');
+  });
 });