Changeset 18752 in josm for trunk/src/org/openstreetmap

Timestamp:

2023-06-13T21:32:22+02:00 (13 months ago)

Author:

taylor.smock

Message:

Fix #9446: Show progress on upload validation (patch updated by gaben)

This does the following:

• Use ValidationTask in more locations (ValidatorCLI and ValidateUploadHook specifically)
• Add an async method for checking upload conditions

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/UploadAction.java (modified) (8 diffs)
• actions/upload/ValidateUploadHook.java (modified) (5 diffs)
• data/validation/ValidationTask.java (modified) (7 diffs)
• data/validation/ValidatorCLI.java (modified) (9 diffs)
• gui/io/SaveLayersDialog.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/actions/UploadAction.java

@@ -12 +12 @@
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.function.Consumer;
 
 import javax.swing.JOptionPane;
@@ -37 +41 @@
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.JosmRuntimeException;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -43 +48 @@
 /**
  * Action that opens a connection to the osm server and uploads all changes.
- *
+ * <p>
  * A dialog is displayed asking the user to specify a rectangle to grab.
  * The url and account settings from the preferences are used.
- *
- * If the upload fails this action offers various options to resolve conflicts.
+ * <p>
+ * If the upload fails, this action offers various options to resolve conflicts.
  *
  * @author imi
@@ -56 +61 @@
      * when the user wants to upload data. Plugins can insert their own hooks here
      * if they want to be able to veto an upload.
-     *
+     * <p>
      * Be default, the standard upload dialog is the only element in the list.
      * Plugins should normally insert their code before that, so that the upload
@@ -195 +200 @@
 
     /**
-     * Check whether the preconditions are met to upload data in <code>apiData</code>.
-     * Makes sure upload is allowed, primitives in <code>apiData</code> don't participate in conflicts and
+     * Check whether the preconditions are met to upload data in {@code apiData}.
+     * Makes sure upload is allowed, primitives in {@code apiData} don't participate in conflicts and
      * runs the installed {@link UploadHook}s.
      *
@@ -204 +209 @@
      */
     public static boolean checkPreUploadConditions(AbstractModifiableLayer layer, APIDataSet apiData) {
+        try {
+            return checkPreUploadConditionsAsync(layer, apiData, null).get();
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new JosmRuntimeException(e);
+        } catch (ExecutionException e) {
+            throw new JosmRuntimeException(e);
+        }
+    }
+
+    /**
+     * Check whether the preconditions are met to upload data in {@code apiData}.
+     * Makes sure upload is allowed, primitives in {@code apiData} don't participate in conflicts and
+     * runs the installed {@link UploadHook}s.
+     *
+     * @param layer the source layer of the data to be uploaded
+     * @param apiData the data to be uploaded
+     * @param onFinish {@code true} if the preconditions are met; {@code false}, otherwise
+     * @return A future that completes when the checks are finished
+     * @since 18752
+     */
+    private static Future<Boolean> checkPreUploadConditionsAsync(AbstractModifiableLayer layer, APIDataSet apiData, Consumer<Boolean> onFinish) {
+        final CompletableFuture<Boolean> future = new CompletableFuture<>();
+        if (onFinish != null) {
+            future.thenAccept(onFinish);
+        }
         if (layer.isUploadDiscouraged() && warnUploadDiscouraged(layer)) {
-            return false;
-        }
-        if (layer instanceof OsmDataLayer) {
+            future.complete(false);
+        } else if (layer instanceof OsmDataLayer) {
             OsmDataLayer osmLayer = (OsmDataLayer) layer;
             ConflictCollection conflicts = osmLayer.getConflicts();
             if (apiData.participatesInConflict(conflicts)) {
                 alertUnresolvedConflicts(osmLayer);
-                return false;
+                future.complete(false);
             }
         }
         // Call all upload hooks in sequence.
-        // FIXME: this should become an asynchronous task
-        //
-        if (apiData != null) {
-            return UPLOAD_HOOKS.stream().allMatch(hook -> hook.checkUpload(apiData));
-        }
-
-        return true;
+        if (!future.isDone()) {
+            MainApplication.worker.execute(() -> {
+                boolean hooks = true;
+                if (apiData != null) {
+                    hooks = UPLOAD_HOOKS.stream().allMatch(hook -> hook.checkUpload(apiData));
+                }
+                future.complete(hooks);
+            });
+        }
+        return future;
     }
 
@@ -236 +269 @@
             return;
         }
-        if (!checkPreUploadConditions(layer, apiData))
-            return;
+        checkPreUploadConditionsAsync(layer, apiData, passed -> GuiHelper.runInEDT(() -> {
+            if (Boolean.TRUE.equals(passed)) {
+                realUploadData(layer, apiData);
+            } else {
+                new Notification(tr("One of the upload verification processes failed")).show();
+            }
+        }));
+    }
+
+    /**
+     * Uploads data to the OSM API.
+     *
+     * @param layer the source layer for the data to upload
+     * @param apiData the primitives to be added, updated, or deleted
+     */
+    private static void realUploadData(final OsmDataLayer layer, final APIDataSet apiData) {
 
         ChangesetUpdater.check();
@@ -284 +331 @@
                     uploadStrategySpecification, layer, apiData, cs);
 
-            if (asyncUploadTask.isPresent()) {
-                MainApplication.worker.execute(asyncUploadTask.get());
-            }
+            asyncUploadTask.ifPresent(MainApplication.worker::execute);
         } else {
             MainApplication.worker.execute(new UploadPrimitivesTask(uploadStrategySpecification, layer, apiData, cs));

trunk/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java

@@ -6 +6 @@
 import java.awt.Dimension;
 import java.awt.GridBagLayout;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.swing.JPanel;
@@ -15 +15 @@
 import org.openstreetmap.josm.data.APIDataSet;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
 import org.openstreetmap.josm.data.validation.OsmValidator;
-import org.openstreetmap.josm.data.validation.Severity;
-import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.data.validation.TestError;
+import org.openstreetmap.josm.data.validation.ValidationTask;
 import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor;
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.MainApplication;
-import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.gui.dialogs.validator.ValidatorTreePanel;
-import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.layer.ValidatorLayer;
 import org.openstreetmap.josm.gui.util.GuiHelper;
@@ -34 +30 @@
  * The action that does the validate thing.
  * <p>
- * This action iterates through all active tests and give them the data, so that
+ * This action iterates through all active tests and gives them the data, so that
  * each one can test it.
  *
@@ -45 +41 @@
      * Validate the modified data before uploading
      * @param apiDataSet contains primitives to be uploaded
-     * @return true if upload should continue, else false
+     * @return {@code true} if upload should continue, else false
      */
     @Override
     public boolean checkUpload(APIDataSet apiDataSet) {
-
-        OsmValidator.initializeTests();
-        Collection<Test> tests = OsmValidator.getEnabledTests(true);
-        if (tests.isEmpty())
-            return true;
-
+        AtomicBoolean returnCode = new AtomicBoolean();
         AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
         v.visit(apiDataSet.getPrimitivesToAdd());
-        Collection<OsmPrimitive> selection = v.visit(apiDataSet.getPrimitivesToUpdate());
+        Collection<OsmPrimitive> visited = v.visit(apiDataSet.getPrimitivesToUpdate());
+        OsmValidator.initializeTests();
+        new ValidationTask(errors -> {
+            if (errors.stream().allMatch(TestError::isIgnored)) {
+                returnCode.set(true);
+            } else {
+                // Unfortunately, the progress monitor is not "finished" until after `finish` is called, so we will
+                // have a ProgressMonitor open behind the error screen. Fortunately, the error screen appears in front
+                // of the progress monitor.
+                GuiHelper.runInEDTAndWait(() -> returnCode.set(displayErrorScreen(errors)));
+            }
+        }, null, OsmValidator.getEnabledTests(true), visited, null, true).run();
 
-        List<TestError> errors = new ArrayList<>(30);
-        for (Test test : tests) {
-            test.setBeforeUpload(true);
-            test.setPartialSelection(true);
-            test.startTest(null);
-            test.visit(selection);
-            test.endTest();
-            if (ValidatorPrefHelper.PREF_OTHER.get() && ValidatorPrefHelper.PREF_OTHER_UPLOAD.get()) {
-                errors.addAll(test.getErrors());
-            } else {
-                for (TestError e : test.getErrors()) {
-                    if (e.getSeverity() != Severity.OTHER) {
-                        errors.add(e);
-                    }
-                }
-            }
-            test.clear();
-            test.setBeforeUpload(false);
-        }
-
-        if (Boolean.TRUE.equals(ValidatorPrefHelper.PREF_USE_IGNORE.get())) {
-            errors.forEach(TestError::updateIgnored);
-        }
-
-        OsmDataLayer editLayer = MainApplication.getLayerManager().getEditLayer();
-        if (editLayer != null) {
-            editLayer.validationErrors.clear();
-            editLayer.validationErrors.addAll(errors);
-        }
-        MapFrame map = MainApplication.getMap();
-        if (map != null) {
-            map.validatorDialog.tree.setErrors(errors);
-        }
-        if (errors.stream().allMatch(TestError::isIgnored))
-            return true;
-
-        return displayErrorScreen(errors);
+        return returnCode.get();
     }
 
@@ -102 +68 @@
      * give the user the possibility to cancel the upload.
      * @param errors The errors displayed in the screen
-     * @return <code>true</code>, if the upload should continue. <code>false</code>
-     *          if the user requested cancel.
+     * @return {@code true}, if the upload should continue.<br>
+     *         {@code false}, if the user requested cancel.
      */
     private static boolean displayErrorScreen(List<TestError> errors) {

trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.awt.GraphicsEnvironment;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
 
 import javax.swing.JOptionPane;
@@ -26 +29 @@
  */
 public class ValidationTask extends PleaseWaitRunnable {
+    private final Consumer<List<TestError>> onFinish;
     private Collection<Test> tests;
     private final Collection<OsmPrimitive> validatedPrimitives;
     private final Collection<OsmPrimitive> formerValidatedPrimitives;
+    private final boolean beforeUpload;
     private boolean canceled;
     private List<TestError> errors;
+    private BiConsumer<ValidationTask, Test> testConsumer;
 
     /**
@@ -45 +51 @@
     }
 
-    protected ValidationTask(ProgressMonitor progressMonitor,
-                             Collection<Test> tests,
-                             Collection<OsmPrimitive> validatedPrimitives,
-                             Collection<OsmPrimitive> formerValidatedPrimitives) {
-        super(tr("Validating"), progressMonitor, false /*don't ignore exceptions */);
+    /**
+     * Constructs a new {@code ValidationTask}
+     *
+     * @param onFinish                  called when the tests are finished
+     * @param progressMonitor           the progress monitor to update with test progress
+     * @param tests                     the tests to run
+     * @param validatedPrimitives       the collection of primitives to validate.
+     * @param formerValidatedPrimitives the last collection of primitives being validates. May be null.
+     * @param beforeUpload              {@code true} if this is being run prior to upload
+     * @since 18752
+     */
+    public ValidationTask(Consumer<List<TestError>> onFinish,
+            ProgressMonitor progressMonitor,
+            Collection<Test> tests,
+            Collection<OsmPrimitive> validatedPrimitives,
+            Collection<OsmPrimitive> formerValidatedPrimitives,
+            boolean beforeUpload) {
+        super(tr("Validating"),
+                progressMonitor != null ? progressMonitor : new PleaseWaitProgressMonitor(tr("Validating")),
+                false /*don't ignore exceptions */);
+        this.onFinish = onFinish;
         this.validatedPrimitives = validatedPrimitives;
         this.formerValidatedPrimitives = formerValidatedPrimitives;
         this.tests = tests;
+        this.beforeUpload = beforeUpload;
+    }
+
+    protected ValidationTask(ProgressMonitor progressMonitor,
+            Collection<Test> tests,
+            Collection<OsmPrimitive> validatedPrimitives,
+            Collection<OsmPrimitive> formerValidatedPrimitives) {
+        this(null, progressMonitor, tests, validatedPrimitives, formerValidatedPrimitives, false);
     }
 
@@ -64 +94 @@
         if (canceled) return;
 
-        // update GUI on Swing EDT
-        GuiHelper.runInEDT(() -> {
-            MapFrame map = MainApplication.getMap();
-            map.validatorDialog.unfurlDialog();
-            map.validatorDialog.tree.setErrors(errors);
-            //FIXME: nicer way to find / invalidate the corresponding error layer
-            MainApplication.getLayerManager().getLayersOfType(ValidatorLayer.class).forEach(ValidatorLayer::invalidate);
-            if (!errors.isEmpty()) {
-                OsmValidator.initializeErrorLayer();
-            }
-        });
+        if (!GraphicsEnvironment.isHeadless() && MainApplication.getMap() != null) {
+            // update GUI on Swing EDT
+            GuiHelper.runInEDT(() -> {
+                MapFrame map = MainApplication.getMap();
+                map.validatorDialog.unfurlDialog();
+                map.validatorDialog.tree.setErrors(errors);
+                //FIXME: nicer way to find / invalidate the corresponding error layer
+                MainApplication.getLayerManager().getLayersOfType(ValidatorLayer.class).forEach(ValidatorLayer::invalidate);
+                if (!errors.isEmpty()) {
+                    OsmValidator.initializeErrorLayer();
+                }
+            });
+        }
+        if (this.onFinish != null) {
+            this.onFinish.accept(this.errors);
+        }
     }
 
@@ -89 +124 @@
             testCounter++;
             getProgressMonitor().setCustomText(tr("Test {0}/{1}: Starting {2}", testCounter, tests.size(), test.getName()));
-            test.setBeforeUpload(false);
+            test.setBeforeUpload(this.beforeUpload);
             test.setPartialSelection(formerValidatedPrimitives != null);
             test.startTest(getProgressMonitor().createSubTaskMonitor(validatedPrimitives.size(), false));
@@ -95 +130 @@
             test.endTest();
             errors.addAll(test.getErrors());
+            if (this.testConsumer != null) {
+                this.testConsumer.accept(this, test);
+            }
             test.clear();
+            test.setBeforeUpload(false);
         }
         tests = null;
@@ -125 +164 @@
         return errors;
     }
+
+    /**
+     * A test consumer to avoid filling up memory. A test consumer <i>may</i> remove tests it has consumed.
+     * @param testConsumer The consumer which takes a {@link ValidationTask} ({@code this}) and the test that finished.
+     * @since 18752
+     */
+    public void setTestConsumer(BiConsumer<ValidationTask, Test> testConsumer) {
+        this.testConsumer = testConsumer;
+    }
 }

trunk/src/org/openstreetmap/josm/data/validation/ValidatorCLI.java

@@ -11 +11 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -26 +27 @@
 import java.util.stream.Collectors;
 
+import jakarta.json.JsonObject;
 import org.apache.commons.compress.utils.FileNameUtils;
 import org.openstreetmap.josm.actions.ExtensionFileFilter;
@@ -177 +179 @@
             for (String inputFile : this.input) {
                 if (inputFile.endsWith(".validator.mapcss")) {
-                    this.processValidatorFile(inputFile);
+                    processValidatorFile(inputFile);
                 } else if (inputFile.endsWith(".mapcss")) {
-                    this.processMapcssFile(inputFile);
+                    processMapcssFile(inputFile);
                 } else {
                     this.processFile(inputFile);
@@ -198 +200 @@
      * @throws ParseException if the file does not match the mapcss syntax
      */
-    private void processMapcssFile(final String inputFile) throws ParseException {
+    private static void processMapcssFile(final String inputFile) throws ParseException {
         final MapCSSStyleSource styleSource = new MapCSSStyleSource(new File(inputFile).toURI().getPath(), inputFile, inputFile);
         styleSource.loadStyleSource();
@@ -215 +217 @@
      * @throws ParseException if the file does not match the validator mapcss syntax
      */
-    private void processValidatorFile(final String inputFile) throws ParseException, IOException {
+    private static void processValidatorFile(final String inputFile) throws ParseException, IOException {
         // Check asserts
         Config.getPref().putBoolean("validator.check_assert_local_rules", true);
@@ -257 +259 @@
         try {
             Logging.info(task);
-            OsmValidator.initializeTests();
             dataLayer = MainApplication.getLayerManager().getLayersOfType(OsmDataLayer.class)
                     .stream().filter(layer -> inputFileFile.equals(layer.getAssociatedFile()))
@@ -270 +271 @@
                 }
             }
-            Collection<Test> tests = OsmValidator.getEnabledTests(false);
-            if (Files.isRegularFile(Paths.get(outputFile)) && !Files.deleteIfExists(Paths.get(outputFile))) {
+            Path path = Paths.get(outputFile);
+            if (path.toFile().isFile() && !Files.deleteIfExists(path)) {
                 Logging.error("Could not delete {0}, attempting to append", outputFile);
             }
             GeoJSONMapRouletteWriter geoJSONMapRouletteWriter = new GeoJSONMapRouletteWriter(dataSet);
-            try (OutputStream fileOutputStream = Files.newOutputStream(Paths.get(outputFile))) {
-                tests.parallelStream().forEach(test -> runTest(test, geoJSONMapRouletteWriter, fileOutputStream, dataSet));
+            OsmValidator.initializeTests();
+
+            try (OutputStream fileOutputStream = Files.newOutputStream(path)) {
+                // The first writeErrors catches anything that was written, for whatever reason. This is probably never
+                // going to be called.
+                ValidationTask validationTask = new ValidationTask(errors -> writeErrors(geoJSONMapRouletteWriter, fileOutputStream, errors),
+                        progressMonitorFactory.get(), OsmValidator.getEnabledTests(false),
+                        dataSet.allPrimitives(), Collections.emptyList(), false);
+                // This avoids keeping errors in memory
+                validationTask.setTestConsumer((t, test) -> {
+                    writeErrors(geoJSONMapRouletteWriter, fileOutputStream, test.getErrors());
+                    t.getErrors().removeIf(test.getErrors()::contains);
+                });
+                validationTask.run();
             }
         } finally {
@@ -283 +296 @@
             }
             Logging.info(stopwatch.toString(task));
+        }
+    }
+
+    private void writeErrors(GeoJSONMapRouletteWriter geoJSONMapRouletteWriter, OutputStream fileOutputStream,
+            Collection<TestError> errors) {
+        for (TestError error : errors) {
+            Optional<JsonObject> object = geoJSONMapRouletteWriter.write(error);
+            if (object.isPresent()) {
+                try {
+                    writeToFile(fileOutputStream, object.get().toString().getBytes(StandardCharsets.UTF_8));
+                } catch (IOException e) {
+                    throw new JosmRuntimeException(e);
+                }
+            }
         }
     }
@@ -300 +327 @@
         }
         return FileNameUtils.getBaseName(FileNameUtils.getBaseName(inputString)) + ".geojson";
-    }
-
-    /**
-     * Run a test
-     * @param test The test to run
-     * @param geoJSONMapRouletteWriter The object to use to create challenges
-     * @param fileOutputStream The location to write data to
-     * @param dataSet The dataset to check
-     */
-    private void runTest(final Test test, final GeoJSONMapRouletteWriter geoJSONMapRouletteWriter,
-            final OutputStream fileOutputStream, DataSet dataSet) {
-        test.startTest(progressMonitorFactory.get());
-        test.visit(dataSet.allPrimitives());
-        test.endTest();
-        test.getErrors().stream().map(geoJSONMapRouletteWriter::write)
-                .filter(Optional::isPresent).map(Optional::get)
-                .map(jsonObject -> jsonObject.toString().getBytes(StandardCharsets.UTF_8)).forEach(bytes -> {
-                    try {
-                        writeToFile(fileOutputStream, bytes);
-                    } catch (IOException e) {
-                        throw new JosmRuntimeException(e);
-                    }
-                });
-        test.clear();
     }
 

trunk/src/org/openstreetmap/josm/gui/io/SaveLayersDialog.java

@@ -64 +64 @@
 /**
  * Dialog that pops up when the user closes a layer with modified data.
- *
- * It asks for confirmation that all modification should be discarded and offers
+ * <p>
+ * It asks for confirmation that all modifications should be discarded and offer
  * to save the layers to file or upload to server, depending on the type of layer.
  */
@@ -450 +450 @@
                     closeDialog();
                 }
-            } catch (UserCancelException ignore) {
-                Logging.trace(ignore);
+            } catch (UserCancelException userCancelException) {
+                Logging.trace(userCancelException);
             }
         }
@@ -557 +557 @@
                 AbstractModifiableLayer layer = layerInfo.getLayer();
                 if (canceled) {
+                    GuiHelper.runInEDTAndWait(() -> model.setUploadState(layer, UploadOrSaveState.CANCELED));
+                    continue;
+                }
+                GuiHelper.runInEDTAndWait(() -> monitor.subTask(tr("Preparing layer ''{0}'' for upload ...", layerInfo.getName())));
+
+                // checkPreUploadConditions must not be run in the EDT to avoid deadlocks
+                if (!UploadAction.checkPreUploadConditions(layer)) {
+                    GuiHelper.runInEDTAndWait(() -> model.setUploadState(layer, UploadOrSaveState.FAILED));
+                    continue;
+                }
+
+                GuiHelper.runInEDTAndWait(() -> uploadLayersUploadModelStateOnFinish(layer));
+                currentTask = null;
+            }
+        }
+
+        /**
+         * Update the {@link #model} state on upload finish
+         * @param layer The layer that has been saved
+         */
+        private void uploadLayersUploadModelStateOnFinish(AbstractModifiableLayer layer) {
+            AbstractUploadDialog dialog = layer.getUploadDialog();
+            if (dialog != null) {
+                dialog.setVisible(true);
+                if (dialog.isCanceled()) {
                     model.setUploadState(layer, UploadOrSaveState.CANCELED);
-                    continue;
-                }
-                monitor.subTask(tr("Preparing layer ''{0}'' for upload ...", layerInfo.getName()));
-
-                if (!UploadAction.checkPreUploadConditions(layer)) {
-                    model.setUploadState(layer, UploadOrSaveState.FAILED);
-                    continue;
-                }
-
-                AbstractUploadDialog dialog = layer.getUploadDialog();
-                if (dialog != null) {
-                    dialog.setVisible(true);
-                    if (dialog.isCanceled()) {
-                        model.setUploadState(layer, UploadOrSaveState.CANCELED);
-                        continue;
-                    }
-                    dialog.rememberUserInput();
-                }
-
-                currentTask = layer.createUploadTask(monitor);
-                if (currentTask == null) {
-                    model.setUploadState(layer, UploadOrSaveState.FAILED);
-                    continue;
-                }
-                Future<?> currentFuture = worker.submit(currentTask);
-                try {
-                    // wait for the asynchronous task to complete
-                    currentFuture.get();
-                } catch (CancellationException e) {
-                    Logging.trace(e);
-                    model.setUploadState(layer, UploadOrSaveState.CANCELED);
-                } catch (InterruptedException | ExecutionException e) {
-                    Logging.error(e);
-                    model.setUploadState(layer, UploadOrSaveState.FAILED);
-                    ExceptionDialogUtil.explainException(e);
-                }
-                if (currentTask.isCanceled()) {
-                    model.setUploadState(layer, UploadOrSaveState.CANCELED);
-                } else if (currentTask.isFailed()) {
-                    Logging.error(currentTask.getLastException());
-                    ExceptionDialogUtil.explainException(currentTask.getLastException());
-                    model.setUploadState(layer, UploadOrSaveState.FAILED);
-                } else {
-                    model.setUploadState(layer, UploadOrSaveState.OK);
-                }
-                currentTask = null;
+                    return;
+                }
+                dialog.rememberUserInput();
+            }
+
+            currentTask = layer.createUploadTask(monitor);
+            if (currentTask == null) {
+                model.setUploadState(layer, UploadOrSaveState.FAILED);
+                return;
+            }
+            Future<?> currentFuture = worker.submit(currentTask);
+            try {
+                // wait for the asynchronous task to complete
+                currentFuture.get();
+            } catch (CancellationException e) {
+                Logging.trace(e);
+                model.setUploadState(layer, UploadOrSaveState.CANCELED);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                Logging.error(e);
+                model.setUploadState(layer, UploadOrSaveState.FAILED);
+                ExceptionDialogUtil.explainException(e);
+            } catch (ExecutionException e) {
+                Logging.error(e);
+                model.setUploadState(layer, UploadOrSaveState.FAILED);
+                ExceptionDialogUtil.explainException(e);
+            }
+            if (currentTask.isCanceled()) {
+                model.setUploadState(layer, UploadOrSaveState.CANCELED);
+            } else if (currentTask.isFailed()) {
+                Logging.error(currentTask.getLastException());
+                ExceptionDialogUtil.explainException(currentTask.getLastException());
+                model.setUploadState(layer, UploadOrSaveState.FAILED);
+            } else {
+                model.setUploadState(layer, UploadOrSaveState.OK);
             }
         }
@@ -673 +687 @@
         @Override
         public void run() {
+            GuiHelper.runInEDTAndWait(() -> model.setMode(SaveLayersModel.Mode.UPLOADING_AND_SAVING));
+            // We very specifically do not want to block the EDT or the worker thread when validating
+            List<SaveLayerInfo> toUpload = model.getLayersToUpload();
+            if (!toUpload.isEmpty()) {
+                uploadLayers(toUpload);
+            }
             GuiHelper.runInEDTAndWait(() -> {
-                model.setMode(SaveLayersModel.Mode.UPLOADING_AND_SAVING);
-                List<SaveLayerInfo> toUpload = model.getLayersToUpload();
-                if (!toUpload.isEmpty()) {
-                    uploadLayers(toUpload);
-                }
                 List<SaveLayerInfo> toSave = model.getLayersToSave();
                 if (!toSave.isEmpty()) {