Browse Source

Refactor internal processing of HealthChecks:
* replace HealthCheckTask by Check class (not Wrapping TaskAPI)
* replace Task-API and BatchService by CheckExecutor (using CompletionStage-API)
* adjust other classes

Armin Schrenk 3 years ago
parent
commit
6250f3d89c

+ 0 - 35
src/main/java/org/cryptomator/ui/health/BatchService.java

@@ -1,35 +0,0 @@
-package org.cryptomator.ui.health;
-
-import com.google.common.base.Preconditions;
-import com.google.common.base.Suppliers;
-import dagger.Lazy;
-
-import javax.inject.Inject;
-import javafx.concurrent.Service;
-import javafx.concurrent.Task;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.concurrent.ExecutorService;
-import java.util.function.Supplier;
-
-public class BatchService extends Service<Void> {
-
-	private final Iterator<HealthCheckTask> remainingTasks;
-
-	public BatchService(Iterable<HealthCheckTask> tasks) {
-		this.remainingTasks = tasks.iterator();
-	}
-
-	@Override
-	protected Task<Void> createTask() {
-		Preconditions.checkState(remainingTasks.hasNext(), "No remaining tasks");
-		return remainingTasks.next();
-	}
-
-	@Override
-	protected void succeeded() {
-		if (remainingTasks.hasNext()) {
-			this.restart();
-		}
-	}
-}

+ 101 - 0
src/main/java/org/cryptomator/ui/health/Check.java

@@ -0,0 +1,101 @@
+package org.cryptomator.ui.health;
+
+import org.cryptomator.cryptofs.health.api.HealthCheck;
+
+import javafx.beans.Observable;
+import javafx.beans.binding.BooleanBinding;
+import javafx.beans.property.BooleanProperty;
+import javafx.beans.property.ObjectProperty;
+import javafx.beans.property.SimpleBooleanProperty;
+import javafx.beans.property.SimpleObjectProperty;
+import javafx.collections.FXCollections;
+import javafx.collections.ObservableList;
+import java.util.MissingResourceException;
+import java.util.ResourceBundle;
+
+public class Check {
+
+	private static final String LOCALIZE_PREFIX = "health.";
+
+	private final HealthCheck check;
+	private final ResourceBundle resourceBundle;
+
+	private final BooleanProperty chosenForExecution = new SimpleBooleanProperty(false);
+	private final ObjectProperty<CheckState> state = new SimpleObjectProperty<>(CheckState.RUNNABLE);
+	private final ObservableList<Result> results = FXCollections.observableArrayList(Result::observables);
+	private final ObjectProperty<Throwable> error = new SimpleObjectProperty<>(null);
+	private final BooleanBinding isInReRunState = state.isNotEqualTo(CheckState.RUNNING).or(state.isNotEqualTo(CheckState.SCHEDULED));
+
+	Check(HealthCheck check, ResourceBundle resourceBundle) {
+		this.check = check;
+		this.resourceBundle = resourceBundle;
+	}
+
+	String getLocalizedName() {
+		try {
+			return resourceBundle.getString(LOCALIZE_PREFIX+check.identifier());
+		} catch (MissingResourceException e){
+			return check.identifier();
+		}
+	}
+
+	HealthCheck getHealthCheck() {
+		return check;
+	}
+
+	BooleanProperty chosenForExecutionProperty() {
+		return chosenForExecution;
+	}
+
+	boolean isChosenForExecution() {
+		return chosenForExecution.get();
+	}
+
+	ObjectProperty stateProperty() {
+		return state;
+	}
+
+	CheckState getState() {
+		return state.get();
+	}
+
+	void setState(CheckState newState) {
+		state.set(newState);
+	}
+
+	ObjectProperty errorProperty() {
+		return error;
+	}
+
+	Throwable getError() {
+		return error.get();
+	}
+
+	void setError(Throwable t) {
+		error.set(t);
+	}
+
+	boolean isInReRunState() {
+		return isInReRunState.get();
+	}
+
+	enum CheckState {
+		RUNNABLE,
+		SCHEDULED,
+		RUNNING,
+		WITH_CRITICALS, //TODO: maybe the highest result represnt by property and only use one state
+		WITH_WARNINGS,
+		ALL_GOOD,
+		SKIPPED,
+		ERROR,
+		CANCELLED;
+	}
+
+	ObservableList<Result> getResults() {
+		return results;
+	}
+
+	Observable[] observables() {
+		return new Observable[]{chosenForExecution, state, results, error};
+	}
+}

+ 12 - 23
src/main/java/org/cryptomator/ui/health/CheckDetailController.java

@@ -12,7 +12,6 @@ import javafx.beans.binding.Binding;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.value.ObservableValue;
 import javafx.collections.FXCollections;
-import javafx.concurrent.Worker;
 import javafx.fxml.FXML;
 import javafx.scene.control.ListView;
 import java.time.Duration;
@@ -24,9 +23,8 @@ import java.util.stream.Stream;
 public class CheckDetailController implements FxController {
 
 	private final EasyObservableList<Result> results;
-	private final OptionalBinding<Worker.State> taskState;
+	private final OptionalBinding<Check.CheckState> taskState;
 	private final Binding<String> taskName;
-	private final Binding<String> taskDuration;
 	private final Binding<Boolean> taskRunning;
 	private final Binding<Boolean> taskScheduled;
 	private final Binding<Boolean> taskFinished;
@@ -43,31 +41,30 @@ public class CheckDetailController implements FxController {
 	private Subscription resultSubscription;
 
 	@Inject
-	public CheckDetailController(ObjectProperty<HealthCheckTask> selectedTask, ResultListCellFactory resultListCellFactory, ResourceBundle resourceBundle) {
+	public CheckDetailController(ObjectProperty<Check> selectedTask, ResultListCellFactory resultListCellFactory, ResourceBundle resourceBundle) {
 		this.resultListCellFactory = resultListCellFactory;
 		this.resourceBundle = resourceBundle;
 		this.results = EasyBind.wrapList(FXCollections.observableArrayList());
-		this.taskState = EasyBind.wrapNullable(selectedTask).mapObservable(HealthCheckTask::stateProperty);
-		this.taskName = EasyBind.wrapNullable(selectedTask).map(HealthCheckTask::getTitle).orElse("");
-		this.taskDuration = EasyBind.wrapNullable(selectedTask).mapObservable(HealthCheckTask::durationInMillisProperty).orElse(-1L).map(this::millisToReadAbleDuration);
-		this.taskRunning = EasyBind.wrapNullable(selectedTask).mapObservable(HealthCheckTask::runningProperty).orElse(false); //TODO: DOES NOT WORK
-		this.taskScheduled = taskState.map(Worker.State.SCHEDULED::equals).orElse(false);
-		this.taskNotStarted = taskState.map(Worker.State.READY::equals).orElse(false);
-		this.taskSucceeded = taskState.map(Worker.State.SUCCEEDED::equals).orElse(false);
-		this.taskFailed = taskState.map(Worker.State.FAILED::equals).orElse(false);
-		this.taskCancelled = taskState.map(Worker.State.CANCELLED::equals).orElse(false);
+		this.taskState = EasyBind.wrapNullable(selectedTask).mapObservable(Check::stateProperty);
+		this.taskName = EasyBind.wrapNullable(selectedTask).map(Check::getLocalizedName).orElse("");
+		this.taskRunning = taskState.map(Check.CheckState.RUNNING::equals).orElse(false);
+		this.taskScheduled = taskState.map(Check.CheckState.SCHEDULED::equals).orElse(false);
+		this.taskNotStarted = taskState.map(Check.CheckState.SKIPPED::equals).orElse(false);
+		this.taskSucceeded = taskState.map(state -> state == Check.CheckState.ALL_GOOD || state == Check.CheckState.WITH_WARNINGS || state == Check.CheckState.WITH_CRITICALS).orElse(false);
+		this.taskFailed = taskState.map(Check.CheckState.ERROR::equals).orElse(false);
+		this.taskCancelled = taskState.map(Check.CheckState.CANCELLED::equals).orElse(false);
 		this.taskFinished = EasyBind.combine(taskSucceeded, taskFailed, taskCancelled, (a, b, c) -> a || b || c);
 		this.countOfWarnSeverity = results.reduce(countSeverity(DiagnosticResult.Severity.WARN));
 		this.countOfCritSeverity = results.reduce(countSeverity(DiagnosticResult.Severity.CRITICAL));
 		selectedTask.addListener(this::selectedTaskChanged);
 	}
 
-	private void selectedTaskChanged(ObservableValue<? extends HealthCheckTask> observable, HealthCheckTask oldValue, HealthCheckTask newValue) {
+	private void selectedTaskChanged(ObservableValue<? extends Check> observable, Check oldValue, Check newValue) {
 		if (resultSubscription != null) {
 			resultSubscription.unsubscribe();
 		}
 		if (newValue != null) {
-			resultSubscription = EasyBind.bindContent(results, newValue.results());
+			resultSubscription = EasyBind.bindContent(results, newValue.getResults());
 		}
 	}
 
@@ -91,14 +88,6 @@ public class CheckDetailController implements FxController {
 		return taskName;
 	}
 
-	public String getTaskDuration() {
-		return taskDuration.getValue();
-	}
-
-	public Binding<String> taskDurationProperty() {
-		return taskDuration;
-	}
-
 	public long getCountOfWarnSeverity() {
 		return countOfWarnSeverity.getValue().longValue();
 	}

+ 129 - 0
src/main/java/org/cryptomator/ui/health/CheckExecutor.java

@@ -0,0 +1,129 @@
+package org.cryptomator.ui.health;
+
+import com.google.common.base.Preconditions;
+import org.cryptomator.common.vaults.Vault;
+import org.cryptomator.cryptofs.VaultConfig;
+import org.cryptomator.cryptofs.health.api.DiagnosticResult;
+import org.cryptomator.cryptofs.health.api.HealthCheck;
+import org.cryptomator.cryptolib.api.CryptorProvider;
+import org.cryptomator.cryptolib.api.Masterkey;
+
+import javax.inject.Inject;
+import javafx.application.Platform;
+import java.nio.file.Path;
+import java.security.SecureRandom;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
+
+@HealthCheckScoped
+public class CheckExecutor {
+
+	private final Path vaultPath;
+	private final SecureRandom csprng;
+	private final Masterkey masterkey;
+	private final VaultConfig vaultConfig;
+	private final ExecutorService sequentialExecutor;
+
+	private volatile boolean isCanceled;
+
+
+	@Inject
+	public CheckExecutor(@HealthCheckWindow Vault vault, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, SecureRandom csprng) {
+		this.vaultPath = vault.getPath();
+		this.masterkey = masterkeyRef.get();
+		this.vaultConfig = vaultConfigRef.get();
+		this.csprng = csprng;
+		this.sequentialExecutor = Executors.newSingleThreadExecutor();
+	}
+
+	public synchronized CompletionStage<Void> executeBatch(List<Check> checks) {
+		isCanceled = false;
+		var scheduledChecks = checks.stream().map(this::execute).toArray(CompletableFuture[]::new);
+		return CompletableFuture.allOf(scheduledChecks);
+	}
+
+	//@formatter:off
+	private CompletionStage<Void> execute(Check check) {
+		Preconditions.checkArgument(check.isInReRunState());
+		return CompletableFuture.runAsync(() -> check.setState(Check.CheckState.SCHEDULED), Platform::runLater)
+			.thenApplyAsync(ignored -> {
+					if (isCanceled) {
+						throw new CancellationException();
+					}
+					Platform.runLater(() -> check.setState(Check.CheckState.RUNNING)); //must be set within the lambda
+					var seenSeverities = EnumSet.noneOf(DiagnosticResult.Severity.class); //used due to efficiency and compactness
+					check(check.getHealthCheck(), diagnosis -> {
+						seenSeverities.add(diagnosis.getSeverity());
+						Platform.runLater(() -> check.getResults().add(Result.create(diagnosis))); //observableLists need to be changed on FXThread
+						if (isCanceled) {
+							throw new CancellationException(); //hacky workaround to stop the check. DO NOT catch this exception (might be wrapped!)
+						}
+					});
+					return determineHighesSeverity(seenSeverities); },
+				sequentialExecutor)
+			.handleAsync((maxSeenSeverity, throwable) -> {
+					var endState = determineEndState(maxSeenSeverity,throwable);
+					check.setState(endState);
+					if( endState != Check.CheckState.CANCELLED) { //canceling throws exception
+						check.setError(throwable);
+					}
+					return null; },
+				Platform::runLater);
+	}
+	//@formatter:on
+
+	private DiagnosticResult.Severity determineHighesSeverity(Set<DiagnosticResult.Severity> seenSeverities) {
+		if (seenSeverities.contains(DiagnosticResult.Severity.CRITICAL)) {
+			return DiagnosticResult.Severity.CRITICAL;
+		} else if (seenSeverities.contains(DiagnosticResult.Severity.WARN)) {
+			return DiagnosticResult.Severity.WARN;
+		} else {
+			return DiagnosticResult.Severity.GOOD;
+		}
+	}
+
+	private Check.CheckState determineEndState(DiagnosticResult.Severity severity, Throwable t) {
+		if (isCanceled) {
+			//we do not check any exception, because CancellationExc might be wrapped
+			return Check.CheckState.CANCELLED;
+		} else if (t != null) {
+			return Check.CheckState.ERROR;
+		} else if (severity == DiagnosticResult.Severity.GOOD) {
+			return Check.CheckState.ALL_GOOD;
+		} else if (severity == DiagnosticResult.Severity.WARN) {
+			return Check.CheckState.WITH_WARNINGS;
+		} else {
+			return Check.CheckState.WITH_CRITICALS;
+		}
+	}
+
+	private void check(HealthCheck healthCheck, Consumer<DiagnosticResult> diagnosisConsumer) {
+		try (var masterkeyClone = masterkey.clone(); //
+			 var cryptor = CryptorProvider.forScheme(vaultConfig.getCipherCombo()).provide(masterkeyClone, csprng)) {
+			healthCheck.check(vaultPath, vaultConfig, masterkeyClone, cryptor, diagnosisConsumer);
+		} catch (Exception e) {
+			throw new CheckFailedException(e);
+		}
+	}
+
+	public synchronized void cancel() {
+		isCanceled = true;
+	}
+
+	public static class CheckFailedException extends CompletionException {
+
+		private CheckFailedException(Throwable cause) {
+			super(cause);
+		}
+	}
+
+}

+ 21 - 24
src/main/java/org/cryptomator/ui/health/CheckListCell.java

@@ -1,20 +1,18 @@
 package org.cryptomator.ui.health;
 
-import org.cryptomator.cryptofs.health.api.DiagnosticResult;
+import com.tobiasdiez.easybind.EasyBind;
 import org.cryptomator.ui.controls.FontAwesome5Icon;
 import org.cryptomator.ui.controls.FontAwesome5IconView;
 
 import javafx.beans.binding.Bindings;
-import javafx.concurrent.Worker;
 import javafx.geometry.Insets;
 import javafx.geometry.Pos;
 import javafx.scene.Node;
 import javafx.scene.control.CheckBox;
 import javafx.scene.control.ContentDisplay;
 import javafx.scene.control.ListCell;
-import java.util.function.Predicate;
 
-class CheckListCell extends ListCell<HealthCheckTask> {
+class CheckListCell extends ListCell<Check> {
 
 	private final FontAwesome5IconView stateIcon = new FontAwesome5IconView();
 	private CheckBox checkBox = new CheckBox();
@@ -24,14 +22,18 @@ class CheckListCell extends ListCell<HealthCheckTask> {
 		setAlignment(Pos.CENTER_LEFT);
 		setContentDisplay(ContentDisplay.LEFT);
 		getStyleClass().add("label");
+		EasyBind.includeWhen(stateIcon.getStyleClass(), "glyph-icon-muted", stateIcon.glyphProperty().isEqualTo(FontAwesome5Icon.INFO_CIRCLE));
+		EasyBind.includeWhen(stateIcon.getStyleClass(), "glyph-icon-primary", stateIcon.glyphProperty().isEqualTo(FontAwesome5Icon.CHECK));
+		EasyBind.includeWhen(stateIcon.getStyleClass(), "glyph-icon-orange", stateIcon.glyphProperty().isEqualTo(FontAwesome5Icon.EXCLAMATION_TRIANGLE));
+		EasyBind.includeWhen(stateIcon.getStyleClass(), "glyph-icon-red", stateIcon.glyphProperty().isEqualTo(FontAwesome5Icon.TIMES));
 	}
 
 	@Override
-	protected void updateItem(HealthCheckTask item, boolean empty) {
+	protected void updateItem(Check item, boolean empty) {
 		super.updateItem(item, empty);
 		if (item != null) {
-			setText(item.getTitle());
-			graphicProperty().bind(Bindings.createObjectBinding(() -> graphicForState(item.getState()), item.stateProperty()));
+			setText(item.getLocalizedName());
+			graphicProperty().bind(EasyBind.map(item.stateProperty(),this::chooseNodeFromState));
 			stateIcon.glyphProperty().bind(Bindings.createObjectBinding(() -> glyphForState(item), item.stateProperty()));
 			checkBox.selectedProperty().bindBidirectional(item.chosenForExecutionProperty());
 		} else {
@@ -42,30 +44,25 @@ class CheckListCell extends ListCell<HealthCheckTask> {
 		}
 	}
 
-	private Node graphicForState(Worker.State state) {
-		return switch (state) {
-			case READY -> checkBox;
-			case SCHEDULED, RUNNING, FAILED, CANCELLED, SUCCEEDED -> stateIcon;
-		};
+	// see getGlyph() for relevant glyphs:
+	private Node chooseNodeFromState(Check.CheckState state) {
+		if (state == Check.CheckState.RUNNABLE) {
+			return checkBox;
+		} else {
+			return stateIcon;
+		}
 	}
 
-	private FontAwesome5Icon glyphForState(HealthCheckTask item) {
+	private FontAwesome5Icon glyphForState(Check item) {
 		return switch (item.getState()) {
-			case READY -> FontAwesome5Icon.COG; //just a placeholder
+			case RUNNABLE, SKIPPED -> null;
 			case SCHEDULED -> FontAwesome5Icon.CLOCK;
 			case RUNNING -> FontAwesome5Icon.SPINNER;
-			case FAILED -> FontAwesome5Icon.EXCLAMATION_TRIANGLE;
+			case ERROR -> FontAwesome5Icon.TIMES;
 			case CANCELLED -> FontAwesome5Icon.BAN;
-			case SUCCEEDED -> checkFoundProblems(item) ? FontAwesome5Icon.EXCLAMATION_TRIANGLE : FontAwesome5Icon.CHECK;
-		};
-	}
-
-	private boolean checkFoundProblems(HealthCheckTask item) {
-		Predicate<DiagnosticResult.Severity> isProblem = severity -> switch (severity) {
-			case WARN, CRITICAL -> true;
-			case INFO, GOOD -> false;
+			case ALL_GOOD -> FontAwesome5Icon.CHECK;
+			case WITH_WARNINGS, WITH_CRITICALS -> FontAwesome5Icon.EXCLAMATION_TRIANGLE;
 		};
-		return item.results().stream().map(Result::diagnosis).map(DiagnosticResult::getSeverity).anyMatch(isProblem);
 	}
 
 }

+ 37 - 64
src/main/java/org/cryptomator/ui/health/CheckListController.java

@@ -1,8 +1,6 @@
 package org.cryptomator.ui.health;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicates;
-import com.tobiasdiez.easybind.EasyBind;
 import dagger.Lazy;
 import org.cryptomator.ui.common.ErrorComponent;
 import org.cryptomator.ui.common.FxController;
@@ -10,18 +8,13 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
-import javafx.beans.binding.Binding;
 import javafx.beans.binding.Bindings;
 import javafx.beans.binding.BooleanBinding;
 import javafx.beans.binding.IntegerBinding;
-import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ObjectProperty;
-import javafx.beans.property.SimpleBooleanProperty;
-import javafx.beans.property.SimpleObjectProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.collections.transformation.FilteredList;
-import javafx.concurrent.Worker;
 import javafx.event.ActionEvent;
 import javafx.fxml.FXML;
 import javafx.scene.control.CheckBox;
@@ -29,91 +22,79 @@ import javafx.scene.control.ListView;
 import javafx.stage.Stage;
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
-import java.util.concurrent.ExecutorService;
 
 @HealthCheckScoped
 public class CheckListController implements FxController {
 
 	private static final Logger LOG = LoggerFactory.getLogger(CheckListController.class);
-	private static final Set<Worker.State> END_STATES = Set.of(Worker.State.FAILED, Worker.State.CANCELLED, Worker.State.SUCCEEDED);
 
 	private final Stage window;
-	private final ObservableList<HealthCheckTask> tasks;
-	private final FilteredList<HealthCheckTask> chosenTasks;
+	private final ObservableList<Check> checks;
+	private final CheckExecutor checkExecutor;
+	private final FilteredList<Check> chosenChecks;
 	private final ReportWriter reportWriter;
-	private final ExecutorService executorService;
-	private final ObjectProperty<HealthCheckTask> selectedTask;
+	private final ObjectProperty<Check> selectedCheck;
+	private final BooleanBinding mainRunStarted; //TODO: rerunning not considered for now
+	private final BooleanBinding somethingsRunning;
 	private final Lazy<ErrorComponent.Builder> errorComponentBuilder;
-	private final SimpleObjectProperty<Worker<?>> runningTask;
-	private final Binding<Boolean> running;
-	private final Binding<Boolean> finished;
 	private final IntegerBinding chosenTaskCount;
 	private final BooleanBinding anyCheckSelected;
-	private final BooleanProperty showResultScreen;
 
 	/* FXML */
-	public ListView<HealthCheckTask> checksListView;
+	public ListView<Check> checksListView;
 
 	@Inject
-	public CheckListController(@HealthCheckWindow Stage window, Lazy<List<HealthCheckTask>> tasks, ReportWriter reportWriteTask, ObjectProperty<HealthCheckTask> selectedTask, ExecutorService executorService, Lazy<ErrorComponent.Builder> errorComponentBuilder) {
+	public CheckListController(@HealthCheckWindow Stage window, List<Check> checks, CheckExecutor checkExecutor, ReportWriter reportWriteTask, ObjectProperty<Check> selectedCheck, Lazy<ErrorComponent.Builder> errorComponentBuilder) {
 		this.window = window;
-		this.tasks = FXCollections.observableList(tasks.get(), HealthCheckTask::observables);
-		this.chosenTasks = this.tasks.filtered(HealthCheckTask::isChosenForExecution);
+		this.checks = FXCollections.observableList(checks, Check::observables);
+		this.checkExecutor = checkExecutor;
+		this.chosenChecks = this.checks.filtered(Check::isChosenForExecution);
 		this.reportWriter = reportWriteTask;
-		this.executorService = executorService;
-		this.selectedTask = selectedTask;
+		this.selectedCheck = selectedCheck;
 		this.errorComponentBuilder = errorComponentBuilder;
-		this.runningTask = new SimpleObjectProperty<>();
-		this.running = EasyBind.wrapNullable(runningTask).mapObservable(Worker::runningProperty).orElse(false);
-		this.finished = EasyBind.wrapNullable(runningTask).mapObservable(Worker::stateProperty).map(END_STATES::contains).orElse(false);
-		this.chosenTaskCount = Bindings.size(this.chosenTasks);
-		this.anyCheckSelected = selectedTask.isNotNull();
-		this.showResultScreen = new SimpleBooleanProperty(false);
+		this.chosenTaskCount = Bindings.size(this.chosenChecks);
+		this.mainRunStarted = Bindings.isEmpty(this.checks.filtered(c -> c.getState() == Check.CheckState.RUNNABLE));
+		this.somethingsRunning = Bindings.isNotEmpty(this.checks.filtered(c -> c.getState() == Check.CheckState.SCHEDULED || c.getState() == Check.CheckState.RUNNING));
+		this.anyCheckSelected = selectedCheck.isNotNull();
 	}
 
 	@FXML
 	public void initialize() {
-		checksListView.setItems(tasks);
+		checksListView.setItems(checks);
 		checksListView.setCellFactory(view -> new CheckListCell());
-		selectedTask.bind(checksListView.getSelectionModel().selectedItemProperty());
+		selectedCheck.bind(checksListView.getSelectionModel().selectedItemProperty());
 	}
 
 	@FXML
 	public void toggleSelectAll(ActionEvent event) {
 		if (event.getSource() instanceof CheckBox c) {
-			tasks.forEach(t -> t.chosenForExecutionProperty().set(c.isSelected()));
+			checks.forEach(t -> t.chosenForExecutionProperty().set(c.isSelected()));
 		}
 	}
 
 	@FXML
 	public void runSelectedChecks() {
-		Preconditions.checkState(runningTask.get() == null);
-
-		// prevent further interaction by cancelling non-chosen tasks:
-		tasks.filtered(Predicates.not(chosenTasks::contains)).forEach(HealthCheckTask::cancel);
-
-		// run chosen tasks:
-		var batchService = new BatchService(chosenTasks);
-		batchService.setExecutor(executorService);
-		batchService.start();
-		runningTask.set(batchService);
-		showResultScreen.set(true);
-		checksListView.getSelectionModel().select(chosenTasks.get(0));
+		Preconditions.checkState(!mainRunStarted.get());
+		Preconditions.checkState(!somethingsRunning.get());
+		Preconditions.checkState(!chosenChecks.isEmpty());
+
+		checks.filtered(c -> !c.isChosenForExecution()).forEach(c -> c.setState(Check.CheckState.SKIPPED));
+		checkExecutor.executeBatch(chosenChecks);
+		checksListView.getSelectionModel().select(chosenChecks.get(0));
 		checksListView.refresh();
 		window.sizeToScene();
 	}
 
 	@FXML
-	public synchronized void cancelCheck() {
-		Preconditions.checkState(runningTask.get() != null);
-		runningTask.get().cancel();
+	public synchronized void cancelRun() {
+		Preconditions.checkState(runningProperty().get());
+		checkExecutor.cancel();
 	}
 
 	@FXML
 	public void exportResults() {
 		try {
-			reportWriter.writeReport(tasks);
+			reportWriter.writeReport(chosenChecks);
 		} catch (IOException e) {
 			LOG.error("Failed to write health check report.", e);
 			errorComponentBuilder.get().cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene();
@@ -122,19 +103,11 @@ public class CheckListController implements FxController {
 
 	/* Getter/Setter */
 	public boolean isRunning() {
-		return running.getValue();
-	}
-
-	public Binding<Boolean> runningProperty() {
-		return running;
-	}
-
-	public boolean isFinished() {
-		return finished.getValue();
+		return somethingsRunning.getValue();
 	}
 
-	public Binding<Boolean> finishedProperty() {
-		return finished;
+	public BooleanBinding runningProperty() {
+		return somethingsRunning;
 	}
 
 	public boolean isAnyCheckSelected() {
@@ -145,12 +118,12 @@ public class CheckListController implements FxController {
 		return anyCheckSelected;
 	}
 
-	public boolean getShowResultScreen() {
-		return showResultScreen.get();
+	public boolean isMainRunStarted() {
+		return mainRunStarted.get();
 	}
 
-	public BooleanProperty showResultScreenProperty() {
-		return showResultScreen;
+	public BooleanBinding mainRunStartedProperty() {
+		return mainRunStarted;
 	}
 
 	public int getChosenTaskCount() {

+ 3 - 12
src/main/java/org/cryptomator/ui/health/HealthCheckModule.java

@@ -26,8 +26,6 @@ import javafx.beans.value.ChangeListener;
 import javafx.scene.Scene;
 import javafx.stage.Modality;
 import javafx.stage.Stage;
-import java.security.SecureRandom;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -51,21 +49,14 @@ abstract class HealthCheckModule {
 
 	@Provides
 	@HealthCheckScoped
-	static Collection<HealthCheck> provideAvailableHealthChecks() {
-		return HealthCheck.allChecks();
-	}
-
-	@Provides
-	@HealthCheckScoped
-	static ObjectProperty<HealthCheckTask> provideSelectedHealthCheckTask() {
+	static ObjectProperty<Check> provideSelectedCheck() {
 		return new SimpleObjectProperty<>();
 	}
 
-	/* Only inject with Lazy-Wrapper!*/
 	@Provides
 	@HealthCheckScoped
-	static List<HealthCheckTask> provideAvailableHealthCheckTasks(Collection<HealthCheck> availableHealthChecks, @HealthCheckWindow Vault vault, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, SecureRandom csprng, ResourceBundle resourceBundle) {
-		return availableHealthChecks.stream().map(check -> new HealthCheckTask(vault.getPath(), vaultConfigRef.get(), masterkeyRef.get(), csprng, check, resourceBundle)).toList();
+	static List<Check> provideAvailableChecks(ResourceBundle bundle) {
+		return HealthCheck.allChecks().stream().map(hc -> new Check(hc, bundle)).toList();
 	}
 
 	@Provides

+ 0 - 114
src/main/java/org/cryptomator/ui/health/HealthCheckTask.java

@@ -1,114 +0,0 @@
-package org.cryptomator.ui.health;
-
-import org.cryptomator.cryptofs.VaultConfig;
-import org.cryptomator.cryptofs.health.api.DiagnosticResult;
-import org.cryptomator.cryptofs.health.api.HealthCheck;
-import org.cryptomator.cryptolib.api.CryptorProvider;
-import org.cryptomator.cryptolib.api.Masterkey;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import javafx.application.Platform;
-import javafx.beans.Observable;
-import javafx.beans.property.BooleanProperty;
-import javafx.beans.property.LongProperty;
-import javafx.beans.property.SimpleBooleanProperty;
-import javafx.beans.property.SimpleLongProperty;
-import javafx.collections.FXCollections;
-import javafx.collections.ObservableList;
-import javafx.concurrent.Task;
-import java.nio.file.Path;
-import java.security.SecureRandom;
-import java.time.Duration;
-import java.time.Instant;
-import java.util.MissingResourceException;
-import java.util.Objects;
-import java.util.ResourceBundle;
-import java.util.concurrent.CancellationException;
-
-class HealthCheckTask extends Task<Void> {
-
-	private static final Logger LOG = LoggerFactory.getLogger(HealthCheckTask.class);
-
-	private final Path vaultPath;
-	private final VaultConfig vaultConfig;
-	private final Masterkey masterkey;
-	private final SecureRandom csprng;
-	private final HealthCheck check;
-	private final ObservableList<Result> results;
-	private final LongProperty durationInMillis;
-	private final BooleanProperty chosenForExecution;
-
-	public HealthCheckTask(Path vaultPath, VaultConfig vaultConfig, Masterkey masterkey, SecureRandom csprng, HealthCheck check, ResourceBundle resourceBundle) {
-		this.vaultPath = Objects.requireNonNull(vaultPath);
-		this.vaultConfig = Objects.requireNonNull(vaultConfig);
-		this.masterkey = Objects.requireNonNull(masterkey);
-		this.csprng = Objects.requireNonNull(csprng);
-		this.check = Objects.requireNonNull(check);
-		this.results = FXCollections.observableArrayList(Result::observables);
-		try {
-			updateTitle(resourceBundle.getString("health." + check.identifier()));
-		} catch (MissingResourceException e) {
-			LOG.warn("Missing proper name for health check {}, falling back to default.", check.identifier());
-			updateTitle(check.identifier());
-		}
-		this.durationInMillis = new SimpleLongProperty(-1);
-		this.chosenForExecution = new SimpleBooleanProperty();
-	}
-
-	@Override
-	protected Void call() {
-		Instant start = Instant.now();
-		try (var masterkeyClone = masterkey.clone(); //
-			 var cryptor = CryptorProvider.forScheme(vaultConfig.getCipherCombo()).provide(masterkeyClone, csprng)) {
-			check.check(vaultPath, vaultConfig, masterkeyClone, cryptor, diagnosis -> {
-				if (isCancelled()) {
-					throw new CancellationException();
-				}
-				Platform.runLater(() -> results.add(Result.create(diagnosis)));
-			});
-		}
-		Platform.runLater(() -> durationInMillis.set(Duration.between(start, Instant.now()).toMillis()));
-		return null;
-	}
-
-	@Override
-	protected void scheduled() {
-		LOG.info("starting {}", check.identifier());
-	}
-
-	@Override
-	protected void done() {
-		LOG.info("finished {}", check.identifier());
-	}
-
-	/* Getter */
-
-	Observable[] observables() {
-		return new Observable[]{results, chosenForExecution};
-	}
-
-	public ObservableList<Result> results() {
-		return results;
-	}
-
-	public HealthCheck getCheck() {
-		return check;
-	}
-
-	public LongProperty durationInMillisProperty() {
-		return durationInMillis;
-	}
-
-	public long getDurationInMillis() {
-		return durationInMillis.get();
-	}
-
-	public BooleanProperty chosenForExecutionProperty() {
-		return chosenForExecution;
-	}
-
-	public boolean isChosenForExecution() {
-		return chosenForExecution.get();
-	}
-}

+ 17 - 21
src/main/java/org/cryptomator/ui/health/ReportWriter.java

@@ -9,7 +9,6 @@ import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import javafx.application.Application;
-import javafx.concurrent.Worker;
 import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
@@ -30,9 +29,9 @@ public class ReportWriter {
 
 	private static final Logger LOG = LoggerFactory.getLogger(ReportWriter.class);
 	private static final String REPORT_HEADER = """
-			**************************************
-			*   Cryptomator Vault Health Report  *
-			**************************************
+			*******************************************
+			*     Cryptomator Vault Health Report     *
+			*******************************************
 			Analyzed vault: %s (Current name "%s")
 			Vault storage path: %s
 			""";
@@ -58,38 +57,35 @@ public class ReportWriter {
 		this.exportDestination = env.getLogDir().orElse(Path.of(System.getProperty("user.home"))).resolve("healthReport_" + vault.getDisplayName() + "_" + TIME_STAMP.format(Instant.now()) + ".log");
 	}
 
-	protected void writeReport(Collection<HealthCheckTask> tasks) throws IOException {
+	protected void writeReport(Collection<Check> performedChecks) throws IOException {
 		try (var out = Files.newOutputStream(exportDestination, 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) {
-				if (task.getState() == Worker.State.READY) {
-					LOG.debug("Skipping not performed check {}.", task.getCheck().identifier());
-					continue;
-				}
-				writer.write(REPORT_CHECK_HEADER.formatted(task.getCheck().identifier()));
-				switch (task.getState()) {
-					case SUCCEEDED -> {
+			for (var check : performedChecks) {
+				writer.write(REPORT_CHECK_HEADER.formatted(check.getHealthCheck().identifier()));
+				switch (check.getState()) {
+					case ALL_GOOD, WITH_CRITICALS, WITH_WARNINGS -> {
 						writer.write("STATUS: SUCCESS\nRESULTS:\n");
-						for (var result : task.results()) {
+						for (var result : check.getResults()) {
 							writer.write(REPORT_CHECK_RESULT.formatted(result.diagnosis().getSeverity(), result.getDescription()));
 						}
 					}
 					case CANCELLED -> writer.write("STATUS: CANCELED\n");
-					case FAILED -> {
-						writer.write("STATUS: FAILED\nREASON:\n" + task.getCheck().identifier());
-						writer.write(prepareFailureMsg(task));
+					case ERROR -> {
+						writer.write("STATUS: FAILED\nREASON:\n");
+						writer.write(prepareFailureMsg(check));
 					}
-					case RUNNING, SCHEDULED -> throw new IllegalStateException("Checks are still running.");
+					case RUNNABLE, RUNNING, SCHEDULED -> throw new IllegalStateException("Checks are still running.");
+					case SKIPPED -> {} //noop
 				}
 			}
 		}
 		reveal();
 	}
 
-	private String prepareFailureMsg(HealthCheckTask task) {
-		if (task.getException() != null) {
-			return ExceptionUtils.getStackTrace(task.getException()) //
+	private String prepareFailureMsg(Check check) {
+		if (check.getError() != null) {
+			return ExceptionUtils.getStackTrace(check.getError()) //
 					.lines() //
 					.map(line -> "\t\t" + line + "\n") //
 					.collect(Collectors.joining());

+ 1 - 1
src/main/resources/fxml/health_check_details.fxml

@@ -16,7 +16,7 @@
 	<Label text="%health.check.detail.taskScheduled" visible="${controller.taskScheduled}" managed="${controller.taskScheduled}"/>
 	<Label text="%health.check.detail.taskCancelled" visible="${controller.taskCancelled}" managed="${controller.taskCancelled}"/>
 	<Label text="%health.check.detail.taskFailed" visible="${controller.taskFailed}" managed="${controller.taskFailed}"/>
-	<FormattedLabel styleClass="label" format="%health.check.detail.taskSucceeded" arg1="${controller.taskDuration}" visible="${controller.taskSucceeded}" managed="${controller.taskSucceeded}"/>
+	<Label text="%health.check.detail.taskSucceeded" visible="${controller.taskSucceeded}" managed="${controller.taskSucceeded}"/>
 
 	<FormattedLabel styleClass="label" format="%health.check.detail.problemCount" arg1="${controller.countOfWarnSeverity}" arg2="${controller.countOfCritSeverity}" visible="${!controller.taskNotStarted}"
 					 managed="${!controller.taskNotStarted}"	/>

+ 6 - 6
src/main/resources/fxml/health_check_list.fxml

@@ -26,21 +26,21 @@
 		<HBox spacing="12" VBox.vgrow="ALWAYS">
 			<VBox minWidth="80" maxWidth="200" spacing="6" HBox.hgrow="ALWAYS" >
 				<Label fx:id="listHeading" text="%health.checkList.header"/>
-				<CheckBox onAction="#toggleSelectAll" text="%health.checkList.selectAllBox" visible="${!controller.showResultScreen}" managed="${!controller.showResultScreen}" />
+				<CheckBox onAction="#toggleSelectAll" text="%health.checkList.selectAllBox" visible="${!controller.mainRunStarted}" managed="${!controller.mainRunStarted}" />
 				<ListView fx:id="checksListView" VBox.vgrow="ALWAYS"/>
 			</VBox>
-			<StackPane visible="${controller.showResultScreen}" HBox.hgrow="ALWAYS" >
+			<StackPane visible="${controller.mainRunStarted}"  managed="${controller.mainRunStarted}" HBox.hgrow="ALWAYS">
 				<VBox minWidth="300" alignment="CENTER" visible="${!controller.anyCheckSelected}" managed="${!controller.anyCheckSelected}" >
 					<Label text="%health.check.detail.noSelectedCheck" wrapText="true" alignment="CENTER" />
 				</VBox>
-				<fx:include  source="/fxml/health_check_details.fxml" visible="${controller.anyCheckSelected}" managed="${controller.anyCheckSelected}" />
+				<fx:include source="/fxml/health_check_details.fxml" visible="${controller.anyCheckSelected}" managed="${controller.anyCheckSelected}" />
 			</StackPane>
 		</HBox>
 		<ButtonBar buttonMinWidth="120" buttonOrder="+CX">
 			<buttons>
-				<Button text="%generic.button.cancel" ButtonBar.buttonData="CANCEL_CLOSE" onAction="#cancelCheck" disable="${!controller.running}" visible="${controller.showResultScreen}" managed="${controller.showResultScreen}" />
-				<Button text="%health.check.exportBtn" ButtonBar.buttonData="NEXT_FORWARD" defaultButton="true" disable="${!controller.finished}" visible="${controller.showResultScreen}" managed="${controller.showResultScreen}" onAction="#exportResults"/>
-				<Button text="%health.check.runBatchBtn" ButtonBar.buttonData="NEXT_FORWARD" defaultButton="true" onAction="#runSelectedChecks" disable="${controller.chosenTaskCount == ZERO}" visible="${!controller.showResultScreen}" managed="${!controller.showResultScreen}"/>
+				<Button text="%generic.button.cancel" ButtonBar.buttonData="CANCEL_CLOSE" onAction="#cancelRun" disable="${!controller.running}" visible="${controller.mainRunStarted}" managed="${controller.mainRunStarted}" />
+				<Button text="%health.check.exportBtn" ButtonBar.buttonData="NEXT_FORWARD" defaultButton="true" disable="${controller.running}" visible="${controller.mainRunStarted}" managed="${controller.mainRunStarted}" onAction="#exportResults"/>
+				<Button text="%health.check.runBatchBtn" ButtonBar.buttonData="NEXT_FORWARD" defaultButton="true" onAction="#runSelectedChecks" disable="${controller.chosenTaskCount == ZERO}" visible="${!controller.mainRunStarted}" managed="${!controller.mainRunStarted}"/>
 			</buttons>
 		</ButtonBar>
 	</children>

+ 1 - 4
src/main/resources/i18n/strings.properties

@@ -159,15 +159,12 @@ health.check.detail.header=Results of %s
 health.check.detail.taskNotStarted=The check was not selected to run.
 health.check.detail.taskScheduled=The check is scheduled.
 health.check.detail.taskRunning=The check is currently running…
-health.check.detail.taskSucceeded=The check finished successfully after %s.
+health.check.detail.taskSucceeded=The check finished successfully.
 health.check.detail.taskFailed=The check exited due to an error.
 health.check.detail.taskCancelled=The check was cancelled.
 health.check.detail.problemCount=Found %d problems and %d unfixable errors.
 health.check.exportBtn=Export Report
 health.check.fixBtn=Fix
-health.check.detail.hmsFormat= %d hours, %2d minutes and %2d seconds
-health.check.detail.msFormat= %d minutes and %2d seconds
-health.check.detail.sFormat= %d seconds
 ## Checks
 health.org.cryptomator.cryptofs.health.dirid.DirIdCheck=Directory Check