write report synchronously, reducing complexity

main/ui/src/main/java/org/cryptomator/ui/health/CheckListController.java

@@ -2,32 +2,32 @@ package org.cryptomator.ui.health;
 
 import com.google.common.base.Preconditions;
 import com.tobiasdiez.easybind.EasyBind;
-import com.tobiasdiez.easybind.optional.OptionalBinding;
 import dagger.Lazy;
-import org.cryptomator.cryptofs.health.api.DiagnosticResult;
 import org.cryptomator.ui.common.FxController;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import javafx.beans.binding.Binding;
 import javafx.beans.binding.BooleanBinding;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
-import javafx.beans.property.SimpleStringProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.concurrent.Worker;
 import javafx.fxml.FXML;
 import javafx.scene.control.ListView;
-import javafx.scene.control.TableColumn;
-import javafx.scene.control.TableView;
+import java.io.IOException;
 import java.util.Collection;
 import java.util.concurrent.ExecutorService;
 
 @HealthCheckScoped
 public class CheckListController implements FxController {
 
+	private static final Logger LOG = LoggerFactory.getLogger(CheckListController.class);
+
 	private final ObservableList<HealthCheckTask> tasks;
-	private final HealthReportWriteTask reportWriter;
+	private final ReportWriter reportWriter;
 	private final ExecutorService executorService;
 	private final ObjectProperty<HealthCheckTask> selectedTask;
 	private final SimpleObjectProperty<Worker<?>> runningTask;
@@ -38,13 +38,13 @@ public class CheckListController implements FxController {
 	public ListView<HealthCheckTask> checksListView;
 
 	@Inject
-	public CheckListController(Lazy<Collection<HealthCheckTask>> tasks, HealthReportWriteTask reportWriteTask, ObjectProperty<HealthCheckTask> selectedTask, ExecutorService executorService) {
+	public CheckListController(Lazy<Collection<HealthCheckTask>> tasks, ReportWriter reportWriteTask, ObjectProperty<HealthCheckTask> selectedTask, ExecutorService executorService) {
 		this.tasks = FXCollections.observableArrayList(tasks.get());
 		this.reportWriter = reportWriteTask;
 		this.executorService = executorService;
 		this.selectedTask = selectedTask;
 		this.runningTask = new SimpleObjectProperty<>();
-		this.running = EasyBind.wrapNullable(runningTask).map(Worker::isRunning).orElse(false);
+		this.running = EasyBind.wrapNullable(runningTask).mapObservable(Worker::runningProperty).orElse(false);
 		this.anyCheckSelected = selectedTask.isNotNull();
 	}
 
@@ -81,7 +81,11 @@ public class CheckListController implements FxController {
 
 	@FXML
 	public void exportResults() {
-		executorService.execute(reportWriter);
+		try {
+			reportWriter.writeReport(tasks);
+		} catch (IOException e) {
+			LOG.error("Failed to write health check report.", e);
+		}
 	}
 
 	/* Getter/Setter */

main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java

@@ -27,6 +27,7 @@ import javafx.scene.Scene;
 import javafx.stage.Modality;
 import javafx.stage.Stage;
 import java.security.SecureRandom;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;

main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java

@@ -29,9 +29,6 @@ class HealthCheckTask extends Task<Void> {
 	private final HealthCheck check;
 	private final ObservableList<DiagnosticResult> results;
 
-	private final AtomicReference<State> endState;
-	private final AtomicReference<Throwable> exceptionOnDone;
-
 	public HealthCheckTask(Path vaultPath, VaultConfig vaultConfig, Masterkey masterkey, SecureRandom csprng, HealthCheck check) {
 		this.vaultPath = Objects.requireNonNull(vaultPath);
 		this.vaultConfig = Objects.requireNonNull(vaultConfig);
@@ -39,8 +36,6 @@ class HealthCheckTask extends Task<Void> {
 		this.csprng = Objects.requireNonNull(csprng);
 		this.check = Objects.requireNonNull(check);
 		this.results = FXCollections.observableArrayList();
-		this.endState = new AtomicReference<>(null);
-		this.exceptionOnDone = new AtomicReference<>();
 
 		var tmp = check.identifier();
 		updateTitle(tmp.substring(tmp.length() - 10)); //TODO: new method with reliable logic
@@ -79,12 +74,6 @@ class HealthCheckTask extends Task<Void> {
 	@Override
 	protected void done() {
 		LOG.info("finished {}", check.identifier());
-		Platform.runLater(() -> endState.set(getState()));
-	}
-
-	@Override
-	protected void failed() {
-		Platform.runLater(() -> exceptionOnDone.set(getException()));
 	}
 
 	/* Getter */
@@ -96,12 +85,4 @@ class HealthCheckTask extends Task<Void> {
 	public HealthCheck getCheck() {
 		return check;
 	}
-
-	public State getEndState() {
-		return endState.get();
-	}
-
-	public Optional<Throwable> getExceptionOnDone() {
-		return Optional.ofNullable(exceptionOnDone.get());
-	}
 }

main/ui/src/main/java/org/cryptomator/ui/health/HealthReportWriteTask.java

@@ -6,14 +6,13 @@ import org.cryptomator.common.Environment;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.Volume;
 import org.cryptomator.cryptofs.VaultConfig;
-import org.cryptomator.cryptofs.health.api.DiagnosticResult;
 import org.cryptomator.ui.common.HostServiceRevealer;
 
 import javax.inject.Inject;
 import javafx.concurrent.Task;
+import java.io.BufferedWriter;
 import java.io.IOException;
-import java.nio.ByteBuffer;
-import java.nio.channels.ByteChannel;
+import java.io.OutputStreamWriter;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -27,13 +26,13 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 @HealthCheckScoped
-public class HealthReportWriteTask extends Task<Void> {
+public class ReportWriter {
 
 	private static final String REPORT_HEADER = """
 			**************************************
 			*   Cryptomator Vault Health Report  *
 			**************************************
-			Analyzed vault: %s (Current name \"%s\")
+			Analyzed vault: %s (Current name "%s")
 			Vault storage path: %s
 			""";
 	private static final String REPORT_CHECK_HEADER = """
@@ -42,72 +41,59 @@ public class HealthReportWriteTask extends Task<Void> {
 			Check %s
 			------------------------------
 			""";
-	private static final String REPORT_CHECK_RESULT = "%s - %s";
+	private static final String REPORT_CHECK_RESULT = "%8s - %s";
 	private static final DateTimeFormatter TIME_STAMP = DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss").withZone(ZoneId.systemDefault());
 
 	private final Vault vault;
 	private final VaultConfig vaultConfig;
-	private final Lazy<Collection<HealthCheckTask>> tasks;
 	private final Path path;
 	private final HostServiceRevealer revealer;
 
 	@Inject
-	public HealthReportWriteTask(@HealthCheckWindow Vault vault, AtomicReference<VaultConfig> vaultConfigRef, Lazy<Collection<HealthCheckTask>> tasks, Environment env, HostServiceRevealer revealer) {
+	public ReportWriter(@HealthCheckWindow Vault vault, AtomicReference<VaultConfig> vaultConfigRef, Environment env, HostServiceRevealer revealer) {
 		this.vault = vault;
 		this.vaultConfig = Objects.requireNonNull(vaultConfigRef.get());
-		this.tasks = tasks;
 		this.revealer = revealer;
 		this.path = env.getLogDir().orElse(Path.of(System.getProperty("user.home"))).resolve("healthReport_" + vault.getDisplayName() + "_" + TIME_STAMP.format(Instant.now()) + ".log");
 	}
 
-	@Override
-	protected Void call() throws IOException {
-		final var tasks = this.tasks.get();
-		//use file channel, since results can be pretty big
-		try (var channel = Files.newByteChannel(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) {
-			internalWrite(channel, String.format(REPORT_HEADER, vaultConfig.getId(), vault.getDisplayName(), vault.getPath()));
+	protected void writeReport(Collection<HealthCheckTask> tasks) throws IOException {
+		try (var out = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); //
+			 var writer = new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8))) {
+			writer.write(REPORT_HEADER.formatted(vaultConfig.getId(), vault.getDisplayName(), vault.getPath()));
 			for (var task : tasks) {
-				internalWrite(channel, REPORT_CHECK_HEADER, task.getCheck().identifier());
-				final var state = task.getEndState();
-				switch (state) {
+				writer.write(REPORT_CHECK_HEADER.formatted(task.getCheck().identifier()));
+				switch (task.getState()) {
 					case SUCCEEDED -> {
-						internalWrite(channel, "STATUS: SUCCESS\nRESULTS:\n");
+						writer.write("STATUS: SUCCESS\nRESULTS:\n");
 						for (var result : task.results()) {
-							internalWrite(channel, REPORT_CHECK_RESULT, severityToString(result.getServerity()), result);
+							writer.write(REPORT_CHECK_RESULT.formatted(result.getServerity(), result));
 						}
 					}
-					case CANCELLED -> internalWrite(channel, "STATUS: CANCELED\n");
+					case CANCELLED -> writer.write("STATUS: CANCELED\n");
 					case FAILED -> {
-						internalWrite(channel, "STATUS: FAILED\nREASON:\n", task.getCheck().identifier());
-						internalWrite(channel, prepareFailureMsg(task));
+						writer.write("STATUS: FAILED\nREASON:\n" + task.getCheck().identifier());
+						writer.write(prepareFailureMsg(task));
 					}
 					case READY, RUNNING, SCHEDULED -> throw new IllegalStateException("Cannot export unfinished task");
 				}
 			}
 		}
-		return null;
-	}
-
-	private void internalWrite(ByteChannel channel, String s, Object... formatArguments) throws IOException {
-		channel.write(ByteBuffer.wrap(s.formatted(formatArguments).getBytes(StandardCharsets.UTF_8)));
-	}
-
-	private String severityToString(DiagnosticResult.Severity s) {
-		return switch (s) {
-			case GOOD, INFO, WARN -> s.name();
-			case CRITICAL -> "CRIT";
-		};
+		reveal();
 	}
 
 	private String prepareFailureMsg(HealthCheckTask task) {
-		return task.getExceptionOnDone() //
-				.map(t -> ExceptionUtils.getStackTrace(t)).orElse("Unknown reason of failure.") //
-				.lines().map(line -> "\t\t" + line + "\n") //
-				.collect(Collectors.joining());
+		if (task.getException() != null) {
+			return ExceptionUtils.getStackTrace(task.getException()) //
+					.lines() //
+					.map(line -> "\t\t" + line + "\n") //
+					.collect(Collectors.joining());
+		} else {
+			return "Unknown reason of failure.";
+		}
 	}
 
-	@Override
-	protected void succeeded() {
+	private void reveal() {
 		try {
 			revealer.reveal(path);
 		} catch (Volume.VolumeException e) {