diff --git a/src/main/java/org/cryptomator/ui/health/Check.java b/src/main/java/org/cryptomator/ui/health/Check.java index 8e3de3f65..ff9a2fcc5 100644 --- a/src/main/java/org/cryptomator/ui/health/Check.java +++ b/src/main/java/org/cryptomator/ui/health/Check.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.health; +import org.cryptomator.cryptofs.health.api.DiagnosticResult; import org.cryptomator.cryptofs.health.api.HealthCheck; import javafx.beans.Observable; @@ -23,6 +24,7 @@ public class Check { private final BooleanProperty chosenForExecution = new SimpleBooleanProperty(false); private final ObjectProperty state = new SimpleObjectProperty<>(CheckState.RUNNABLE); private final ObservableList results = FXCollections.observableArrayList(Result::observables); + private final ObjectProperty highestResultSeverity = new SimpleObjectProperty<>(null); private final ObjectProperty error = new SimpleObjectProperty<>(null); private final BooleanBinding isInReRunState = state.isNotEqualTo(CheckState.RUNNING).or(state.isNotEqualTo(CheckState.SCHEDULED)); @@ -75,6 +77,18 @@ public class Check { error.set(t); } + ObjectProperty highestResultSeverityProperty() { + return highestResultSeverity; + } + + DiagnosticResult.Severity getHighestResultSeverity() { + return highestResultSeverity.get(); + } + + void setHighestResultSeverity(DiagnosticResult.Severity severity) { + highestResultSeverity.set(severity); + } + boolean isInReRunState() { return isInReRunState.get(); } @@ -83,9 +97,7 @@ public class Check { RUNNABLE, SCHEDULED, RUNNING, - WITH_CRITICALS, //TODO: maybe the highest result represnt by property and only use one state - WITH_WARNINGS, - ALL_GOOD, + SUCCEEDED, SKIPPED, ERROR, CANCELLED; diff --git a/src/main/java/org/cryptomator/ui/health/CheckDetailController.java b/src/main/java/org/cryptomator/ui/health/CheckDetailController.java index b08c32b9e..666ff2ce3 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckDetailController.java +++ b/src/main/java/org/cryptomator/ui/health/CheckDetailController.java @@ -46,7 +46,7 @@ public class CheckDetailController implements FxController { this.checkRunning = checkState.map(Check.CheckState.RUNNING::equals).orElse(false); this.checkScheduled = checkState.map(Check.CheckState.SCHEDULED::equals).orElse(false); this.checkSkipped = checkState.map(Check.CheckState.SKIPPED::equals).orElse(false); - this.checkSucceeded = checkState.map(state -> state == Check.CheckState.ALL_GOOD || state == Check.CheckState.WITH_WARNINGS || state == Check.CheckState.WITH_CRITICALS).orElse(false); + this.checkSucceeded = checkState.map(Check.CheckState.SUCCEEDED::equals).orElse(false); this.checkFailed = checkState.map(Check.CheckState.ERROR::equals).orElse(false); this.checkCancelled = checkState.map(Check.CheckState.CANCELLED::equals).orElse(false); this.checkFinished = EasyBind.combine(checkSucceeded, checkFailed, checkCancelled, (a, b, c) -> a || b || c); diff --git a/src/main/java/org/cryptomator/ui/health/CheckExecutor.java b/src/main/java/org/cryptomator/ui/health/CheckExecutor.java index 738c9f92f..4b082f2a1 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckExecutor.java +++ b/src/main/java/org/cryptomator/ui/health/CheckExecutor.java @@ -1,28 +1,21 @@ 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 javafx.concurrent.Task; import java.nio.file.Path; import java.security.SecureRandom; -import java.util.EnumSet; +import java.util.ArrayDeque; 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.Queue; 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 { @@ -32,8 +25,7 @@ public class CheckExecutor { private final Masterkey masterkey; private final VaultConfig vaultConfig; private final ExecutorService sequentialExecutor; - - private volatile boolean isCanceled; + private final Queue runningTasks; @Inject @@ -43,87 +35,79 @@ public class CheckExecutor { this.vaultConfig = vaultConfigRef.get(); this.csprng = csprng; this.sequentialExecutor = Executors.newSingleThreadExecutor(); + this.runningTasks = new ArrayDeque<>(); } - public synchronized CompletionStage executeBatch(List checks) { - isCanceled = false; - var scheduledChecks = checks.stream().map(this::execute).toArray(CompletableFuture[]::new); - return CompletableFuture.allOf(scheduledChecks); - } - - //@formatter:off - private CompletionStage 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 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 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 executeBatch(List checks) { + checks.stream().map(c -> { + c.setState(Check.CheckState.SCHEDULED); + var task = new CheckTask(c); + runningTasks.add(task); // we need to use CheckTask and not Futures to set state to CANCEL + return task; + }).forEach(sequentialExecutor::submit); } public synchronized void cancel() { - isCanceled = true; - } - - public static class CheckFailedException extends CompletionException { - - private CheckFailedException(Throwable cause) { - super(cause); + while (!runningTasks.isEmpty()) { + runningTasks.remove().cancel(true); } } + private class CheckTask extends Task { + + private Check c; + private DiagnosticResult.Severity highestResultSeverity; + + CheckTask(Check c) { + this.c = c; + } + + @Override + protected Void call() throws Exception { + try (var masterkeyClone = masterkey.clone(); // + var cryptor = CryptorProvider.forScheme(vaultConfig.getCipherCombo()).provide(masterkeyClone, csprng)) { + c.getHealthCheck().check(vaultPath, vaultConfig, masterkeyClone, cryptor, diagnosis -> { + c.getResults().add(Result.create(diagnosis)); + compareAndSetSeverity(diagnosis.getSeverity()); + }); + } + return null; + } + + private void compareAndSetSeverity(DiagnosticResult.Severity newOne) { + if (highestResultSeverity != DiagnosticResult.Severity.CRITICAL && newOne == DiagnosticResult.Severity.CRITICAL) { + highestResultSeverity = DiagnosticResult.Severity.CRITICAL; + } else if (highestResultSeverity != DiagnosticResult.Severity.WARN && newOne == DiagnosticResult.Severity.WARN) { + highestResultSeverity = DiagnosticResult.Severity.WARN; + } else if (highestResultSeverity != DiagnosticResult.Severity.GOOD && newOne == DiagnosticResult.Severity.GOOD) { + highestResultSeverity = DiagnosticResult.Severity.GOOD; + } else { + highestResultSeverity = DiagnosticResult.Severity.INFO; + } + } + + @Override + protected void running() { + c.setState(Check.CheckState.RUNNING); + } + + @Override + protected void cancelled() { + c.setState(Check.CheckState.CANCELLED); + } + + @Override + protected void succeeded() { + c.setState(Check.CheckState.SUCCEEDED); + c.setHighestResultSeverity(highestResultSeverity); + } + + @Override + protected void failed() { + c.setState(Check.CheckState.ERROR); + c.setError(this.getException()); + } + + } + } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/ui/health/CheckListCell.java b/src/main/java/org/cryptomator/ui/health/CheckListCell.java index 240359693..7c5532e04 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckListCell.java +++ b/src/main/java/org/cryptomator/ui/health/CheckListCell.java @@ -1,6 +1,7 @@ package org.cryptomator.ui.health; import com.tobiasdiez.easybind.EasyBind; +import org.cryptomator.cryptofs.health.api.DiagnosticResult; import org.cryptomator.ui.controls.FontAwesome5Icon; import org.cryptomator.ui.controls.FontAwesome5IconView; @@ -34,7 +35,7 @@ class CheckListCell extends ListCell { if (item != null) { setText(item.getLocalizedName()); graphicProperty().bind(EasyBind.map(item.stateProperty(),this::chooseNodeFromState)); - stateIcon.glyphProperty().bind(Bindings.createObjectBinding(() -> glyphForState(item), item.stateProperty())); + stateIcon.glyphProperty().bind(Bindings.createObjectBinding(() -> glyphForState(item), item.stateProperty(), item.highestResultSeverityProperty())); checkBox.selectedProperty().bindBidirectional(item.chosenForExecutionProperty()); } else { graphicProperty().unbind(); @@ -60,8 +61,13 @@ class CheckListCell extends ListCell { case RUNNING -> FontAwesome5Icon.SPINNER; case ERROR -> FontAwesome5Icon.TIMES; case CANCELLED -> FontAwesome5Icon.BAN; - case ALL_GOOD -> FontAwesome5Icon.CHECK; - case WITH_WARNINGS, WITH_CRITICALS -> FontAwesome5Icon.EXCLAMATION_TRIANGLE; + case SUCCEEDED -> { + if (item.getHighestResultSeverity() == DiagnosticResult.Severity.INFO || item.getHighestResultSeverity() == DiagnosticResult.Severity.GOOD) { + yield FontAwesome5Icon.CHECK; + } else { + yield FontAwesome5Icon.EXCLAMATION_TRIANGLE; + } + } }; } diff --git a/src/main/java/org/cryptomator/ui/health/ReportWriter.java b/src/main/java/org/cryptomator/ui/health/ReportWriter.java index 9068676d4..8630f0a77 100644 --- a/src/main/java/org/cryptomator/ui/health/ReportWriter.java +++ b/src/main/java/org/cryptomator/ui/health/ReportWriter.java @@ -64,7 +64,7 @@ public class ReportWriter { for (var check : performedChecks) { writer.write(REPORT_CHECK_HEADER.formatted(check.getHealthCheck().identifier())); switch (check.getState()) { - case ALL_GOOD, WITH_CRITICALS, WITH_WARNINGS -> { + case SUCCEEDED -> { writer.write("STATUS: SUCCESS\nRESULTS:\n"); for (var result : check.getResults()) { writer.write(REPORT_CHECK_RESULT.formatted(result.diagnosis().getSeverity(), result.getDescription())); diff --git a/src/main/resources/fxml/health_check_details.fxml b/src/main/resources/fxml/health_check_details.fxml index f2393a9f2..f13894f8f 100644 --- a/src/main/resources/fxml/health_check_details.fxml +++ b/src/main/resources/fxml/health_check_details.fxml @@ -18,7 +18,7 @@