From 2e52afed5191ede28622dd8cdabd80ce582937b8 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 10 May 2021 13:39:46 +0200 Subject: [PATCH] write report synchronously, reducing complexity --- .../ui/health/CheckListController.java | 22 +++--- .../ui/health/HealthCheckModule.java | 1 + .../ui/health/HealthCheckTask.java | 19 ------ ...ReportWriteTask.java => ReportWriter.java} | 68 ++++++++----------- 4 files changed, 41 insertions(+), 69 deletions(-) rename main/ui/src/main/java/org/cryptomator/ui/health/{HealthReportWriteTask.java => ReportWriter.java} (50%) diff --git a/main/ui/src/main/java/org/cryptomator/ui/health/CheckListController.java b/main/ui/src/main/java/org/cryptomator/ui/health/CheckListController.java index dc340001c..9021e1a55 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/health/CheckListController.java +++ b/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 tasks; - private final HealthReportWriteTask reportWriter; + private final ReportWriter reportWriter; private final ExecutorService executorService; private final ObjectProperty selectedTask; private final SimpleObjectProperty> runningTask; @@ -38,13 +38,13 @@ public class CheckListController implements FxController { public ListView checksListView; @Inject - public CheckListController(Lazy> tasks, HealthReportWriteTask reportWriteTask, ObjectProperty selectedTask, ExecutorService executorService) { + public CheckListController(Lazy> tasks, ReportWriter reportWriteTask, ObjectProperty 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 */ diff --git a/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java b/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java index fa9b69adc..aafb5cb22 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java +++ b/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; diff --git a/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java b/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java index 56bcffc17..2ba16cab2 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java +++ b/main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java @@ -29,9 +29,6 @@ class HealthCheckTask extends Task { private final HealthCheck check; private final ObservableList results; - private final AtomicReference endState; - private final AtomicReference 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 { 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 { @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 { public HealthCheck getCheck() { return check; } - - public State getEndState() { - return endState.get(); - } - - public Optional getExceptionOnDone() { - return Optional.ofNullable(exceptionOnDone.get()); - } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/health/HealthReportWriteTask.java b/main/ui/src/main/java/org/cryptomator/ui/health/ReportWriter.java similarity index 50% rename from main/ui/src/main/java/org/cryptomator/ui/health/HealthReportWriteTask.java rename to main/ui/src/main/java/org/cryptomator/ui/health/ReportWriter.java index 8ed4ded6f..4e03500be 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/health/HealthReportWriteTask.java +++ b/main/ui/src/main/java/org/cryptomator/ui/health/ReportWriter.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 { +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 { 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> tasks; private final Path path; private final HostServiceRevealer revealer; @Inject - public HealthReportWriteTask(@HealthCheckWindow Vault vault, AtomicReference vaultConfigRef, Lazy> tasks, Environment env, HostServiceRevealer revealer) { + public ReportWriter(@HealthCheckWindow Vault vault, AtomicReference 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 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) {