Browse Source

* Made DiagnosticResultAction a "pseudo-singleton" that consumes diagnostic results
* renamed FxmlFile.HEALTH_CHECK to match fxml file name
* added missing DI scopes
* simplified cell layout

Sebastian Stenzel 3 years ago
parent
commit
04e5170403

+ 1 - 1
main/ui/src/main/java/org/cryptomator/ui/common/FxmlFile.java

@@ -12,7 +12,7 @@ public enum FxmlFile {
 	ERROR("/fxml/error.fxml"), //
 	FORGET_PASSWORD("/fxml/forget_password.fxml"), //
 	HEALTH_START("/fxml/health_start.fxml"), //
-	HEALTH_CHECK("/fxml/health_check_list.fxml"), //
+	HEALTH_CHECK_LIST("/fxml/health_check_list.fxml"), //
 	LOCK_FORCED("/fxml/lock_forced.fxml"), //
 	LOCK_FAILED("/fxml/lock_failed.fxml"), //
 	MAIN_WINDOW("/fxml/main_window.fxml"), //

+ 5 - 3
main/ui/src/main/java/org/cryptomator/ui/health/CheckDetailController.java

@@ -2,6 +2,7 @@ package org.cryptomator.ui.health;
 
 import com.tobiasdiez.easybind.EasyBind;
 import com.tobiasdiez.easybind.optional.OptionalBinding;
+import org.cryptomator.cryptofs.health.api.DiagnosticResult;
 import org.cryptomator.ui.common.FxController;
 
 import javax.inject.Inject;
@@ -14,15 +15,16 @@ import javafx.concurrent.Worker;
 import javafx.fxml.FXML;
 import javafx.scene.control.ListView;
 
+@HealthCheckScoped
 public class CheckDetailController implements FxController {
 
-	private final Binding<ObservableList<DiagnosticResultAction>> results;
+	private final Binding<ObservableList<DiagnosticResult>> results;
 	private final OptionalBinding<Worker.State> taskState;
 	private final Binding<String> taskName;
 	private final ResultListCellFactory resultListCellFactory;
 	private final BooleanBinding producingResults;
 
-	public ListView<DiagnosticResultAction> resultsListView;
+	public ListView<DiagnosticResult> resultsListView;
 
 	@Inject
 	public CheckDetailController(ObjectProperty<HealthCheckTask> selectedTask, ResultListCellFactory resultListCellFactory) {
@@ -45,8 +47,8 @@ public class CheckDetailController implements FxController {
 		resultsListView.itemsProperty().bind(results);
 		resultsListView.setCellFactory(resultListCellFactory);
 	}
-	/* Getter/Setter */
 
+	/* Getter/Setter */
 
 	public String getTaskName() {
 		return taskName.getValue();

+ 16 - 8
main/ui/src/main/java/org/cryptomator/ui/health/CheckListCell.java

@@ -6,6 +6,8 @@ import org.cryptomator.ui.controls.FontAwesome5IconView;
 import javafx.beans.value.ObservableValue;
 import javafx.concurrent.Worker;
 import javafx.geometry.Insets;
+import javafx.geometry.Pos;
+import javafx.scene.Node;
 import javafx.scene.control.ContentDisplay;
 import javafx.scene.control.ListCell;
 
@@ -13,25 +15,24 @@ class CheckListCell extends ListCell<HealthCheckTask> {
 
 	private final FontAwesome5IconView stateIcon = new FontAwesome5IconView();
 
-	CheckListCell(){
-		paddingProperty().set(new Insets(6));
+	CheckListCell() {
+		setPadding(new Insets(6));
+		setAlignment(Pos.CENTER_LEFT);
+		setContentDisplay(ContentDisplay.LEFT);
 	}
 
 	@Override
 	protected void updateItem(HealthCheckTask item, boolean empty) {
 		super.updateItem(item, empty);
+
 		if (item != null) {
 			setText(item.getTitle());
 			item.stateProperty().addListener(this::stateChanged);
-			setGraphic(stateIcon);
+			setGraphic(graphicForState(item.getState()));
 			stateIcon.setGlyph(glyphForState(item.getState()));
-			if (item.getState() == Worker.State.READY) {
-				stateIcon.setVisible(false);
-			}
-			setContentDisplay(ContentDisplay.LEFT);
 		} else {
+			setGraphic(null);
 			setText(null);
-			setContentDisplay(ContentDisplay.TEXT_ONLY);
 		}
 	}
 
@@ -40,6 +41,13 @@ class CheckListCell extends ListCell<HealthCheckTask> {
 		stateIcon.setVisible(true);
 	}
 
+	private Node graphicForState(Worker.State state) {
+		return switch (state) {
+			case READY -> null;
+			case SCHEDULED, RUNNING, FAILED, CANCELLED, SUCCEEDED -> stateIcon;
+		};
+	}
+
 	private FontAwesome5Icon glyphForState(Worker.State state) {
 		return switch (state) {
 			case READY -> FontAwesome5Icon.COG; //just a placeholder

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

@@ -27,7 +27,6 @@ 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;
@@ -113,10 +112,10 @@ abstract class HealthCheckModule {
 	}
 
 	@Provides
-	@FxmlScene(FxmlFile.HEALTH_CHECK)
+	@FxmlScene(FxmlFile.HEALTH_CHECK_LIST)
 	@HealthCheckScoped
-	static Scene provideHealthCheckScene(@HealthCheckWindow FxmlLoaderFactory fxmlLoaders) {
-		return fxmlLoaders.createScene(FxmlFile.HEALTH_CHECK);
+	static Scene provideHealthCheckListScene(@HealthCheckWindow FxmlLoaderFactory fxmlLoaders) {
+		return fxmlLoaders.createScene(FxmlFile.HEALTH_CHECK_LIST);
 	}
 
 	@Binds

+ 4 - 5
main/ui/src/main/java/org/cryptomator/ui/health/HealthCheckTask.java

@@ -26,7 +26,7 @@ class HealthCheckTask extends Task<Void> {
 	private final Masterkey masterkey;
 	private final SecureRandom csprng;
 	private final HealthCheck check;
-	private final ObservableList<DiagnosticResultAction> results;
+	private final ObservableList<DiagnosticResult> results;
 
 	public HealthCheckTask(Path vaultPath, VaultConfig vaultConfig, Masterkey masterkey, SecureRandom csprng, HealthCheck check) {
 		this.vaultPath = Objects.requireNonNull(vaultPath);
@@ -35,7 +35,6 @@ class HealthCheckTask extends Task<Void> {
 		this.csprng = Objects.requireNonNull(csprng);
 		this.check = Objects.requireNonNull(check);
 		this.results = FXCollections.observableArrayList();
-
 		updateTitle(getDisplayNameOf(check));
 	}
 
@@ -58,7 +57,7 @@ class HealthCheckTask extends Task<Void> {
 						throw new RuntimeException(e);
 					}
 				}
-				Platform.runLater(() -> results.add(new DiagnosticResultAction(result,vaultPath,vaultConfig, masterkey,csprng))); //FIXME: there can be a lotta results, each with a reference to the master key -> differentiate with severity!
+				Platform.runLater(() -> results.add(result));
 			});
 		}
 		return null;
@@ -76,7 +75,7 @@ class HealthCheckTask extends Task<Void> {
 
 	/* Getter */
 
-	public ObservableList<DiagnosticResultAction> results() {
+	public ObservableList<DiagnosticResult> results() {
 		return results;
 	}
 
@@ -85,7 +84,7 @@ class HealthCheckTask extends Task<Void> {
 	}
 
 	static String getDisplayNameOf(HealthCheck check) {
-		if( check instanceof DirIdCheck) { //TODO: discuss if this should be localized
+		if (check instanceof DirIdCheck) { //TODO: discuss if this should be localized
 			return "DirectoryCheck";
 		} else {
 			return check.identifier();

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

@@ -72,7 +72,7 @@ public class ReportWriter {
 					case SUCCEEDED -> {
 						writer.write("STATUS: SUCCESS\nRESULTS:\n");
 						for (var result : task.results()) {
-							writer.write(REPORT_CHECK_RESULT.formatted(result.getSeverity(), result.getDescription()));
+							writer.write(REPORT_CHECK_RESULT.formatted(result.getServerity(), result.toString()));
 						}
 					}
 					case CANCELLED -> writer.write("STATUS: CANCELED\n");

+ 20 - 20
main/ui/src/main/java/org/cryptomator/ui/health/DiagnosticResultAction.java

@@ -1,47 +1,47 @@
 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.cryptolib.api.Masterkey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import javax.inject.Inject;
 import javafx.scene.control.Alert;
 import java.nio.file.Path;
 import java.security.SecureRandom;
+import java.util.concurrent.atomic.AtomicReference;
 
-class DiagnosticResultAction implements Runnable {
+@HealthCheckScoped
+class ResultFixApplier {
+
+	private static final Logger LOG = LoggerFactory.getLogger(ResultFixApplier.class);
 
-	private final DiagnosticResult result;
 	private final Path vaultPath;
-	private final VaultConfig vaultConfig;
-	private final Masterkey masterkey;
 	private final SecureRandom csprng;
+	private final Masterkey masterkey;
+	private final VaultConfig vaultConfig;
 
-	DiagnosticResultAction(DiagnosticResult result, Path vaultPath, VaultConfig vaultConfig, Masterkey masterkey, SecureRandom csprng) {
-		this.result = result;
-		this.vaultPath = vaultPath;
-		this.vaultConfig = vaultConfig;
-		this.masterkey = masterkey;
+	@Inject
+	public ResultFixApplier(@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;
 	}
 
-	public void run() {
+	public void fix(DiagnosticResult result) {
+		Preconditions.checkArgument(result.getServerity() == DiagnosticResult.Severity.WARN, "Unfixable result");
 		try (var masterkeyClone = masterkey.clone(); //
 			 var cryptor = vaultConfig.getCipherCombo().getCryptorProvider(csprng).withKey(masterkeyClone)) {
 			result.fix(vaultPath, vaultConfig, masterkeyClone, cryptor);
 		} catch (Exception e) {
-			e.printStackTrace();
+			LOG.error("Failed to apply fix", e);
 			Alert alert = new Alert(Alert.AlertType.ERROR, e.getMessage());
 			alert.showAndWait();
 			//TODO: real error/not supported handling
 		}
 	}
-
-	public DiagnosticResult.Severity getSeverity() {
-		return result.getServerity(); //TODO: fix spelling with updated cryptofs release
-	}
-
-	public String getDescription() {
-		return result.toString();
-	}
-
 }

+ 14 - 12
main/ui/src/main/java/org/cryptomator/ui/health/ResultListCellController.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.common.FxController;
 import org.cryptomator.ui.controls.FontAwesome5Icon;
 import org.cryptomator.ui.controls.FontAwesome5IconView;
@@ -13,27 +14,28 @@ import javafx.beans.value.ObservableValue;
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
 
+// unscoped because each cell needs its own controller
 public class ResultListCellController implements FxController {
 
-	private final ObjectProperty<DiagnosticResultAction> result;
+	private final ResultFixApplier fixApplier;
+	private final ObjectProperty<DiagnosticResult> result;
 	private final Binding<String> description;
 
-	@FXML
 	public FontAwesome5IconView iconView;
-	@FXML
 	public Button actionButton;
 
 	@Inject
-	public ResultListCellController() {
+	public ResultListCellController(ResultFixApplier fixApplier) {
 		this.result = new SimpleObjectProperty<>(null);
-		this.description = EasyBind.wrapNullable(result).map(DiagnosticResultAction::getDescription).orElse("");
+		this.description = EasyBind.wrapNullable(result).map(DiagnosticResult::toString).orElse("");
+		this.fixApplier = fixApplier;
 		result.addListener(this::updateCellContent);
 	}
 
-	private void updateCellContent(ObservableValue<? extends DiagnosticResultAction> observable, DiagnosticResultAction oldVal, DiagnosticResultAction newVal) {
+	private void updateCellContent(ObservableValue<? extends DiagnosticResult> observable, DiagnosticResult oldVal, DiagnosticResult newVal) {
 		iconView.getStyleClass().clear();
 		actionButton.setVisible(false);
-		switch (newVal.getSeverity()) {
+		switch (newVal.getServerity()) {
 			case INFO -> {
 				iconView.setGlyph(FontAwesome5Icon.INFO_CIRCLE);
 				iconView.getStyleClass().add("glyph-icon-muted");
@@ -58,21 +60,21 @@ public class ResultListCellController implements FxController {
 	public void runResultAction() {
 		final var realResult = result.get();
 		if (realResult != null) {
-			realResult.run(); //TODO: this hogs currently the JAVAFX thread
+			fixApplier.fix(realResult);
 		}
 	}
-
 	/* Getter & Setter */
 
-	public DiagnosticResultAction getResult() {
+
+	public DiagnosticResult getResult() {
 		return result.get();
 	}
 
-	public void setResult(DiagnosticResultAction result) {
+	public void setResult(DiagnosticResult result) {
 		this.result.set(result);
 	}
 
-	public ObjectProperty<DiagnosticResultAction> resultProperty() {
+	public ObjectProperty<DiagnosticResult> resultProperty() {
 		return result;
 	}
 

+ 6 - 4
main/ui/src/main/java/org/cryptomator/ui/health/ResultListCellFactory.java

@@ -1,6 +1,7 @@
 package org.cryptomator.ui.health;
 
 
+import org.cryptomator.cryptofs.health.api.DiagnosticResult;
 import org.cryptomator.ui.common.FxmlLoaderFactory;
 
 import javax.inject.Inject;
@@ -13,7 +14,8 @@ import javafx.util.Callback;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 
-public class ResultListCellFactory implements Callback<ListView<DiagnosticResultAction>, ListCell<DiagnosticResultAction>> {
+@HealthCheckScoped
+public class ResultListCellFactory implements Callback<ListView<DiagnosticResult>, ListCell<DiagnosticResult>> {
 
 	private final FxmlLoaderFactory fxmlLoaders;
 
@@ -23,7 +25,7 @@ public class ResultListCellFactory implements Callback<ListView<DiagnosticResult
 	}
 
 	@Override
-	public ListCell<DiagnosticResultAction> call(ListView<DiagnosticResultAction> param) {
+	public ListCell<DiagnosticResult> call(ListView<DiagnosticResult> param) {
 		try {
 			FXMLLoader fxmlLoader = fxmlLoaders.load("/fxml/health_result_listcell.fxml");
 			return new ResultListCellFactory.Cell(fxmlLoader.getRoot(), fxmlLoader.getController());
@@ -32,7 +34,7 @@ public class ResultListCellFactory implements Callback<ListView<DiagnosticResult
 		}
 	}
 
-	private static class Cell extends ListCell<DiagnosticResultAction> {
+	private static class Cell extends ListCell<DiagnosticResult> {
 
 		private final Parent node;
 		private final ResultListCellController controller;
@@ -43,7 +45,7 @@ public class ResultListCellFactory implements Callback<ListView<DiagnosticResult
 		}
 
 		@Override
-		protected void updateItem(DiagnosticResultAction item, boolean empty) {
+		protected void updateItem(DiagnosticResult item, boolean empty) {
 			super.updateItem(item, empty);
 			if (item == null || empty) {
 				setText(null);

+ 1 - 1
main/ui/src/main/java/org/cryptomator/ui/health/StartController.java

@@ -42,7 +42,7 @@ public class StartController implements FxController {
 	/* FXML */
 
 	@Inject
-	public StartController(@HealthCheckWindow Vault vault, @HealthCheckWindow Stage window, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, @FxmlScene(FxmlFile.HEALTH_CHECK) Lazy<Scene> checkScene, Lazy<ErrorComponent.Builder> errorComponent) {
+	public StartController(@HealthCheckWindow Vault vault, @HealthCheckWindow Stage window, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference<Masterkey> masterkeyRef, AtomicReference<VaultConfig> vaultConfigRef, @FxmlScene(FxmlFile.HEALTH_CHECK_LIST) Lazy<Scene> checkScene, Lazy<ErrorComponent.Builder> errorComponent) {
 		this.window = window;
 		this.unverifiedVaultConfig = vault.getUnverifiedVaultConfig(); //TODO: prevent workflow at all, if the vault config is emtpy
 		this.keyLoadingStrategy = keyLoadingStrategy;