From 6250f3d89cad8aa158608f8419af816e20658aa5 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 6 Jul 2021 16:33:17 +0200 Subject: [PATCH 01/40] 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 --- .../cryptomator/ui/health/BatchService.java | 35 ----- .../java/org/cryptomator/ui/health/Check.java | 101 ++++++++++++++ .../ui/health/CheckDetailController.java | 35 ++--- .../cryptomator/ui/health/CheckExecutor.java | 129 ++++++++++++++++++ .../cryptomator/ui/health/CheckListCell.java | 45 +++--- .../ui/health/CheckListController.java | 99 +++++--------- .../ui/health/HealthCheckModule.java | 15 +- .../ui/health/HealthCheckTask.java | 114 ---------------- .../cryptomator/ui/health/ReportWriter.java | 38 +++--- .../resources/fxml/health_check_details.fxml | 2 +- .../resources/fxml/health_check_list.fxml | 12 +- src/main/resources/i18n/strings.properties | 5 +- 12 files changed, 327 insertions(+), 303 deletions(-) delete mode 100644 src/main/java/org/cryptomator/ui/health/BatchService.java create mode 100644 src/main/java/org/cryptomator/ui/health/Check.java create mode 100644 src/main/java/org/cryptomator/ui/health/CheckExecutor.java delete mode 100644 src/main/java/org/cryptomator/ui/health/HealthCheckTask.java diff --git a/src/main/java/org/cryptomator/ui/health/BatchService.java b/src/main/java/org/cryptomator/ui/health/BatchService.java deleted file mode 100644 index 40f4e173f..000000000 --- a/src/main/java/org/cryptomator/ui/health/BatchService.java +++ /dev/null @@ -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 { - - private final Iterator remainingTasks; - - public BatchService(Iterable tasks) { - this.remainingTasks = tasks.iterator(); - } - - @Override - protected Task createTask() { - Preconditions.checkState(remainingTasks.hasNext(), "No remaining tasks"); - return remainingTasks.next(); - } - - @Override - protected void succeeded() { - if (remainingTasks.hasNext()) { - this.restart(); - } - } -} diff --git a/src/main/java/org/cryptomator/ui/health/Check.java b/src/main/java/org/cryptomator/ui/health/Check.java new file mode 100644 index 000000000..8e3de3f65 --- /dev/null +++ b/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 state = new SimpleObjectProperty<>(CheckState.RUNNABLE); + private final ObservableList results = FXCollections.observableArrayList(Result::observables); + private final ObjectProperty 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 getResults() { + return results; + } + + Observable[] observables() { + return new Observable[]{chosenForExecution, state, results, error}; + } +} diff --git a/src/main/java/org/cryptomator/ui/health/CheckDetailController.java b/src/main/java/org/cryptomator/ui/health/CheckDetailController.java index 7ccc9e09c..83f5dae4a 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckDetailController.java +++ b/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 results; - private final OptionalBinding taskState; + private final OptionalBinding taskState; private final Binding taskName; - private final Binding taskDuration; private final Binding taskRunning; private final Binding taskScheduled; private final Binding taskFinished; @@ -43,31 +41,30 @@ public class CheckDetailController implements FxController { private Subscription resultSubscription; @Inject - public CheckDetailController(ObjectProperty selectedTask, ResultListCellFactory resultListCellFactory, ResourceBundle resourceBundle) { + public CheckDetailController(ObjectProperty 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 observable, HealthCheckTask oldValue, HealthCheckTask newValue) { + private void selectedTaskChanged(ObservableValue 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 taskDurationProperty() { - return taskDuration; - } - public long getCountOfWarnSeverity() { return countOfWarnSeverity.getValue().longValue(); } diff --git a/src/main/java/org/cryptomator/ui/health/CheckExecutor.java b/src/main/java/org/cryptomator/ui/health/CheckExecutor.java new file mode 100644 index 000000000..738c9f92f --- /dev/null +++ b/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 masterkeyRef, AtomicReference 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 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 cancel() { + isCanceled = true; + } + + public static class CheckFailedException extends CompletionException { + + private CheckFailedException(Throwable cause) { + super(cause); + } + } + +} \ 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 76a8f3c27..240359693 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckListCell.java +++ b/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 { +class CheckListCell extends ListCell { private final FontAwesome5IconView stateIcon = new FontAwesome5IconView(); private CheckBox checkBox = new CheckBox(); @@ -24,14 +22,18 @@ class CheckListCell extends ListCell { 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 { } } - 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; + case ALL_GOOD -> FontAwesome5Icon.CHECK; + case WITH_WARNINGS, WITH_CRITICALS -> FontAwesome5Icon.EXCLAMATION_TRIANGLE; }; } - private boolean checkFoundProblems(HealthCheckTask item) { - Predicate isProblem = severity -> switch (severity) { - case WARN, CRITICAL -> true; - case INFO, GOOD -> false; - }; - return item.results().stream().map(Result::diagnosis).map(DiagnosticResult::getSeverity).anyMatch(isProblem); - } - } diff --git a/src/main/java/org/cryptomator/ui/health/CheckListController.java b/src/main/java/org/cryptomator/ui/health/CheckListController.java index 7710eb14d..fc11917aa 100644 --- a/src/main/java/org/cryptomator/ui/health/CheckListController.java +++ b/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 END_STATES = Set.of(Worker.State.FAILED, Worker.State.CANCELLED, Worker.State.SUCCEEDED); private final Stage window; - private final ObservableList tasks; - private final FilteredList chosenTasks; + private final ObservableList checks; + private final CheckExecutor checkExecutor; + private final FilteredList chosenChecks; private final ReportWriter reportWriter; - private final ExecutorService executorService; - private final ObjectProperty selectedTask; + private final ObjectProperty selectedCheck; + private final BooleanBinding mainRunStarted; //TODO: rerunning not considered for now + private final BooleanBinding somethingsRunning; private final Lazy errorComponentBuilder; - private final SimpleObjectProperty> runningTask; - private final Binding running; - private final Binding finished; private final IntegerBinding chosenTaskCount; private final BooleanBinding anyCheckSelected; - private final BooleanProperty showResultScreen; /* FXML */ - public ListView checksListView; + public ListView checksListView; @Inject - public CheckListController(@HealthCheckWindow Stage window, Lazy> tasks, ReportWriter reportWriteTask, ObjectProperty selectedTask, ExecutorService executorService, Lazy errorComponentBuilder) { + public CheckListController(@HealthCheckWindow Stage window, List checks, CheckExecutor checkExecutor, ReportWriter reportWriteTask, ObjectProperty selectedCheck, Lazy 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); + Preconditions.checkState(!mainRunStarted.get()); + Preconditions.checkState(!somethingsRunning.get()); + Preconditions.checkState(!chosenChecks.isEmpty()); - // 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)); + 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(); + return somethingsRunning.getValue(); } - public Binding runningProperty() { - return running; - } - - public boolean isFinished() { - return finished.getValue(); - } - - public Binding 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() { diff --git a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java b/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java index 78643f011..5a4404fcf 100644 --- a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java +++ b/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 provideAvailableHealthChecks() { - return HealthCheck.allChecks(); - } - - @Provides - @HealthCheckScoped - static ObjectProperty provideSelectedHealthCheckTask() { + static ObjectProperty provideSelectedCheck() { return new SimpleObjectProperty<>(); } - /* Only inject with Lazy-Wrapper!*/ @Provides @HealthCheckScoped - static List provideAvailableHealthCheckTasks(Collection availableHealthChecks, @HealthCheckWindow Vault vault, AtomicReference masterkeyRef, AtomicReference vaultConfigRef, SecureRandom csprng, ResourceBundle resourceBundle) { - return availableHealthChecks.stream().map(check -> new HealthCheckTask(vault.getPath(), vaultConfigRef.get(), masterkeyRef.get(), csprng, check, resourceBundle)).toList(); + static List provideAvailableChecks(ResourceBundle bundle) { + return HealthCheck.allChecks().stream().map(hc -> new Check(hc, bundle)).toList(); } @Provides diff --git a/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java b/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java deleted file mode 100644 index 3efe333b3..000000000 --- a/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java +++ /dev/null @@ -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 { - - 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 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 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(); - } -} diff --git a/src/main/java/org/cryptomator/ui/health/ReportWriter.java b/src/main/java/org/cryptomator/ui/health/ReportWriter.java index 6039901ef..9068676d4 100644 --- a/src/main/java/org/cryptomator/ui/health/ReportWriter.java +++ b/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 tasks) throws IOException { + protected void writeReport(Collection 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()); diff --git a/src/main/resources/fxml/health_check_details.fxml b/src/main/resources/fxml/health_check_details.fxml index 51d9b22ae..198d43cde 100644 --- a/src/main/resources/fxml/health_check_details.fxml +++ b/src/main/resources/fxml/health_check_details.fxml @@ -16,7 +16,7 @@