Refactored "lock all" task

main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java

@@ -1,9 +1,5 @@
 package org.cryptomator.ui.common;
 
-import com.google.common.collect.ImmutableList;
-import javafx.application.Platform;
-import javafx.concurrent.ScheduledService;
-import javafx.concurrent.Service;
 import javafx.concurrent.Task;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultState;
@@ -13,9 +9,13 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
+import java.util.stream.Collectors;
 
 @FxApplicationScoped
 public class VaultService {
@@ -75,36 +75,24 @@ public class VaultService {
 	 * @param forced Whether to attempt a forced lock
 	 */
 	public void lockAll(Collection<Vault> vaults, boolean forced) {
-		Service<Vault> service = createLockAllService(vaults, forced);
-		service.setOnSucceeded(evt -> LOG.info("Locked {}", service.getValue().getDisplayableName()));
-		service.setOnFailed(evt -> LOG.error("Failed to lock vault", evt.getSource().getException()));
-		service.start();
+		executorService.execute(createLockAllTask(vaults, forced));
 	}
 
 	/**
-	 * Creates but doesn't start a lock-all service that can be run on a background thread.
+	 * Creates but doesn't start a lock-all task.
 	 *
 	 * @param vaults The list of vaults to be locked
 	 * @param forced Whether to attempt a forced lock
-	 * @return Service that tries to lock all given vaults and cancels itself automatically when done
+	 * @return Meta-Task that waits until all vaults are locked or fails after the first failure of a subtask
 	 */
-	public Service<Vault> createLockAllService(Collection<Vault> vaults, boolean forced) {
-		Iterator<Vault> iter = ImmutableList.copyOf(vaults).iterator();
-		ScheduledService<Vault> service = new ScheduledService<>() {
-
-			@Override
-			protected Task<Vault> createTask() {
-				assert Platform.isFxApplicationThread();
-				if (iter.hasNext()) {
-					return new LockVaultTask(iter.next(), forced);
-				} else {
-					cancel();
-					return new IllegalStateTask("This task should never be executed.");
-				}
-			}
-		};
-		service.setExecutor(executorService);
-		return service;
+	public Task<Collection<Vault>> createLockAllTask(Collection<Vault> vaults, boolean forced) {
+		List<Task<Vault>> lockTasks = vaults.stream().map(v -> new LockVaultTask(v, forced)).collect(Collectors.toUnmodifiableList());
+		lockTasks.forEach(executorService::execute);
+		Task<Collection<Vault>> task = new WaitForTasksTask(lockTasks);
+		String vaultNames = vaults.stream().map(Vault::getDisplayableName).collect(Collectors.joining(", "));
+		task.setOnSucceeded(evt -> LOG.info("Locked {}", vaultNames));
+		task.setOnFailed(evt -> LOG.error("Failed to lock vaults " + vaultNames, evt.getSource().getException()));
+		return task;
 	}
 
 	private static class RevealVaultTask extends Task<Vault> {
@@ -125,6 +113,38 @@ public class VaultService {
 		}
 	}
 
+	/**
+	 * A task that waits for completion of multiple other tasks
+	 */
+	private static class WaitForTasksTask extends Task<Collection<Vault>> {
+
+		private final Collection<Task<Vault>> startedTasks;
+
+		public WaitForTasksTask(Collection<Task<Vault>> tasks) {
+			this.startedTasks = List.copyOf(tasks);
+		}
+
+		@Override
+		protected Collection<Vault> call() throws Exception {
+			Iterator<Task<Vault>> remainingTasks = startedTasks.iterator();
+			Collection<Vault> completed = new ArrayList<>();
+			try {
+				// wait for all tasks:
+				while (remainingTasks.hasNext()) {
+					Vault lockedVault = remainingTasks.next().get();
+					completed.add(lockedVault);
+				}
+			} catch (ExecutionException e) {
+				// cancel all remaining:
+				while (remainingTasks.hasNext()) {
+					remainingTasks.next().cancel(true);
+				}
+				throw e;
+			}
+			return List.copyOf(completed);
+		}
+	}
+
 	/**
 	 * A task that locks a vault
 	 */
@@ -165,24 +185,6 @@ public class VaultService {
 
 	}
 
-	/**
-	 * A task that throws an IllegalStateException
-	 */
-	private static class IllegalStateTask<V> extends Task<V> {
-
-		private final String message;
 
-		/**
-		 * @param message The message of the IllegalStateException
-		 */
-		public IllegalStateTask(String message) {
-			this.message = message;
-		}
-
-		@Override
-		protected V call() throws IllegalStateException {
-			throw new IllegalStateException(message);
-		}
-	}
 
 }

main/ui/src/main/java/org/cryptomator/ui/quit/QuitController.java

@@ -1,7 +1,7 @@
 package org.cryptomator.ui.quit;
 
 import javafx.collections.ObservableList;
-import javafx.concurrent.Service;
+import javafx.concurrent.Task;
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
 import javafx.scene.control.ContentDisplay;
@@ -14,7 +14,9 @@ import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import java.awt.desktop.QuitResponse;
-import java.util.List;
+import java.util.Collection;
+import java.util.concurrent.ExecutorService;
+import java.util.stream.Collectors;
 
 @QuitScoped
 public class QuitController implements FxController {
@@ -24,14 +26,16 @@ public class QuitController implements FxController {
 	private final Stage window;
 	private final QuitResponse response;
 	private final ObservableList<Vault> unlockedVaults;
+	private final ExecutorService executorService;
 	private final VaultService vaultService;
 	public Button lockAndQuitButton;
 
 	@Inject
-	QuitController(@QuitWindow Stage window, QuitResponse response, ObservableList<Vault> vaults, VaultService vaultService) {
+	QuitController(@QuitWindow Stage window, QuitResponse response, ObservableList<Vault> vaults, ExecutorService executorService, VaultService vaultService) {
 		this.window = window;
 		this.response = response;
 		this.unlockedVaults = vaults.filtered(Vault::isUnlocked);
+		this.executorService = executorService;
 		this.vaultService = vaultService;
 	}
 
@@ -47,24 +51,23 @@ public class QuitController implements FxController {
 		lockAndQuitButton.setDisable(true);
 		lockAndQuitButton.setContentDisplay(ContentDisplay.LEFT);
 
-		Service<Vault> lockAllService = vaultService.createLockAllService(unlockedVaults, false);
-
-		lockAllService.setOnSucceeded(evt -> {
-			LOG.info("Locked {}", lockAllService.getValue().getDisplayableName());
+		Task<Collection<Vault>> lockAllTask = vaultService.createLockAllTask(unlockedVaults, false);
+		lockAllTask.setOnSucceeded(evt -> {
+			LOG.info("Locked {}", lockAllTask.getValue().stream().map(Vault::getDisplayableName).collect(Collectors.joining(", ")));
 			if (unlockedVaults.isEmpty()) {
 				window.close();
 				response.performQuit();
 			}
 		});
-		lockAllService.setOnFailed(evt -> {
-			LOG.warn("Locking failed", lockAllService.getException());
+		lockAllTask.setOnFailed(evt -> {
+			LOG.warn("Locking failed", lockAllTask.getException());
 			lockAndQuitButton.setDisable(false);
 			lockAndQuitButton.setContentDisplay(ContentDisplay.TEXT_ONLY);
 			// TODO: show force lock or force quit scene (and DO NOT cancelQuit() here!)
 			// see https://github.com/cryptomator/cryptomator/blob/1.4.16/main/ui/src/main/java/org/cryptomator/ui/model/Vault.java#L151-L163
 			response.cancelQuit();
 		});
-		lockAllService.start();
+		executorService.execute(lockAllTask);
 	}
 
 }