From cb5d628cfccf42da7f1fe79673841e247a66c5aa Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 9 Jul 2021 13:51:54 +0200 Subject: [PATCH] Refactor health check start: * replace health tab in options by a button in gernal tab * move info text of health tab into start controller * replace lazy loading of config in controller by loading in dagger module * add new scene+controller for failed config loading * don't close options window on health check start --- .../org/cryptomator/common/vaults/Vault.java | 9 +++ .../org/cryptomator/ui/common/FxmlFile.java | 1 + .../ui/health/HealthCheckComponent.java | 24 +++--- .../ui/health/HealthCheckModule.java | 24 ++++++ .../ui/health/StartController.java | 77 ++----------------- .../ui/health/StartFailController.java | 41 ++++++++++ .../GeneralVaultOptionsController.java | 10 ++- .../HealthVaultOptionsController.java | 30 -------- .../vaultoptions/SelectedVaultOptionsTab.java | 5 -- .../vaultoptions/VaultOptionsController.java | 2 - .../ui/vaultoptions/VaultOptionsModule.java | 5 -- src/main/resources/fxml/health_start.fxml | 47 ++++++----- .../resources/fxml/health_start_fail.fxml | 19 +++++ src/main/resources/fxml/vault_options.fxml | 8 -- .../resources/fxml/vault_options_general.fxml | 7 ++ 15 files changed, 161 insertions(+), 148 deletions(-) create mode 100644 src/main/java/org/cryptomator/ui/health/StartFailController.java delete mode 100644 src/main/java/org/cryptomator/ui/vaultoptions/HealthVaultOptionsController.java create mode 100644 src/main/resources/fxml/health_start_fail.fxml diff --git a/src/main/java/org/cryptomator/common/vaults/Vault.java b/src/main/java/org/cryptomator/common/vaults/Vault.java index 02fd03600..74ac7dc40 100644 --- a/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -19,6 +19,7 @@ import org.cryptomator.cryptofs.CryptoFileSystemProperties.FileSystemFlags; import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptofs.VaultConfig; import org.cryptomator.cryptofs.VaultConfig.UnverifiedVaultConfig; +import org.cryptomator.cryptofs.VaultConfigLoadException; import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.MasterkeyLoader; @@ -327,6 +328,14 @@ public class Vault { return stats; } + /** + * Attempts to read the vault config file and parse it without verifying its integrity. + * + * @return an unverified vault config + * @throws VaultConfigLoadException if the read file cannot be properly parsed + * @throws IOException if reading the file fails + * + */ public UnverifiedVaultConfig getUnverifiedVaultConfig() throws IOException { Path configPath = getPath().resolve(org.cryptomator.common.Constants.VAULTCONFIG_FILENAME); String token = Files.readString(configPath, StandardCharsets.US_ASCII); diff --git a/src/main/java/org/cryptomator/ui/common/FxmlFile.java b/src/main/java/org/cryptomator/ui/common/FxmlFile.java index b8d5bbff0..ea0c1ed38 100644 --- a/src/main/java/org/cryptomator/ui/common/FxmlFile.java +++ b/src/main/java/org/cryptomator/ui/common/FxmlFile.java @@ -12,6 +12,7 @@ public enum FxmlFile { ERROR("/fxml/error.fxml"), // FORGET_PASSWORD("/fxml/forget_password.fxml"), // HEALTH_START("/fxml/health_start.fxml"), // + HEALTH_START_FAIL("/fxml/health_start_fail.fxml"), // HEALTH_CHECK_LIST("/fxml/health_check_list.fxml"), // LOCK_FORCED("/fxml/lock_forced.fxml"), // LOCK_FAILED("/fxml/lock_failed.fxml"), // diff --git a/src/main/java/org/cryptomator/ui/health/HealthCheckComponent.java b/src/main/java/org/cryptomator/ui/health/HealthCheckComponent.java index 365ab63df..851499a8c 100644 --- a/src/main/java/org/cryptomator/ui/health/HealthCheckComponent.java +++ b/src/main/java/org/cryptomator/ui/health/HealthCheckComponent.java @@ -4,10 +4,10 @@ import dagger.BindsInstance; import dagger.Lazy; import dagger.Subcomponent; import org.cryptomator.common.vaults.Vault; +import org.cryptomator.cryptofs.VaultConfig; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; -import javax.inject.Named; import javafx.scene.Scene; import javafx.stage.Stage; @@ -15,20 +15,26 @@ import javafx.stage.Stage; @Subcomponent(modules = {HealthCheckModule.class}) public interface HealthCheckComponent { + LoadUnverifiedConfigResult loadConfig(); + @HealthCheckWindow Stage window(); - @Named("windowToClose") - Stage windowToClose(); - @FxmlScene(FxmlFile.HEALTH_START) - Lazy scene(); + Lazy startScene(); + + @FxmlScene(FxmlFile.HEALTH_START_FAIL) + Lazy failScene(); default Stage showHealthCheckWindow() { Stage stage = window(); - stage.setScene(scene().get()); + var unverifiedConf = loadConfig(); + if (unverifiedConf.config() != null) { + stage.setScene(startScene().get()); + } else { + stage.setScene(failScene().get()); + } stage.show(); - windowToClose().close(); return stage; } @@ -38,10 +44,8 @@ public interface HealthCheckComponent { @BindsInstance Builder vault(@HealthCheckWindow Vault vault); - @BindsInstance - Builder windowToClose(@Named("windowToClose") Stage window); - HealthCheckComponent build(); } + record LoadUnverifiedConfigResult(VaultConfig.UnverifiedVaultConfig config, Throwable error) {} } diff --git a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java b/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java index 5a4404fcf..958277bbe 100644 --- a/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java +++ b/src/main/java/org/cryptomator/ui/health/HealthCheckModule.java @@ -26,6 +26,7 @@ import javafx.beans.value.ChangeListener; import javafx.scene.Scene; import javafx.stage.Modality; import javafx.stage.Stage; +import java.io.IOException; import java.util.List; import java.util.Map; import java.util.Optional; @@ -35,6 +36,17 @@ import java.util.concurrent.atomic.AtomicReference; @Module(subcomponents = {KeyLoadingComponent.class}) abstract class HealthCheckModule { + @Provides + @HealthCheckScoped + static HealthCheckComponent.LoadUnverifiedConfigResult provideLoadConfigResult(@HealthCheckWindow Vault vault) { + try { + return new HealthCheckComponent.LoadUnverifiedConfigResult(vault.getUnverifiedVaultConfig(), null); + } catch (IOException e) { + return new HealthCheckComponent.LoadUnverifiedConfigResult(null, e); + } + } + + @Provides @HealthCheckScoped static AtomicReference provideMasterkeyRef() { @@ -103,6 +115,13 @@ abstract class HealthCheckModule { return fxmlLoaders.createScene(FxmlFile.HEALTH_START); } + @Provides + @FxmlScene(FxmlFile.HEALTH_START_FAIL) + @HealthCheckScoped + static Scene provideHealthStartFailScene(@HealthCheckWindow FxmlLoaderFactory fxmlLoaders) { + return fxmlLoaders.createScene(FxmlFile.HEALTH_START_FAIL); + } + @Provides @FxmlScene(FxmlFile.HEALTH_CHECK_LIST) @HealthCheckScoped @@ -115,6 +134,11 @@ abstract class HealthCheckModule { @FxControllerKey(StartController.class) abstract FxController bindStartController(StartController controller); + @Binds + @IntoMap + @FxControllerKey(StartFailController.class) + abstract FxController bindStartFailController(StartFailController controller); + @Binds @IntoMap @FxControllerKey(CheckListController.class) diff --git a/src/main/java/org/cryptomator/ui/health/StartController.java b/src/main/java/org/cryptomator/ui/health/StartController.java index 56368e46a..93b04e5bb 100644 --- a/src/main/java/org/cryptomator/ui/health/StartController.java +++ b/src/main/java/org/cryptomator/ui/health/StartController.java @@ -1,7 +1,7 @@ package org.cryptomator.ui.health; +import com.google.common.base.Preconditions; import dagger.Lazy; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.VaultConfig; import org.cryptomator.cryptofs.VaultConfigLoadException; import org.cryptomator.cryptofs.VaultKeyInvalidException; @@ -18,14 +18,11 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javafx.application.Platform; -import javafx.beans.binding.BooleanBinding; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.fxml.FXML; import javafx.scene.Scene; import javafx.stage.Stage; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; @@ -36,40 +33,26 @@ public class StartController implements FxController { private static final Logger LOG = LoggerFactory.getLogger(StartController.class); - private final Vault vault; private final Stage window; - private final CompletableFuture unverifiedVaultConfig; + private final ObjectProperty unverifiedVaultConfig; private final KeyLoadingStrategy keyLoadingStrategy; private final ExecutorService executor; private final AtomicReference masterkeyRef; private final AtomicReference vaultConfigRef; private final Lazy checkScene; private final Lazy errorComponent; - private final ObjectProperty state = new SimpleObjectProperty<>(State.LOADING); - private final BooleanBinding loading = state.isEqualTo(State.LOADING); - private final BooleanBinding failed = state.isEqualTo(State.FAILED); - private final BooleanBinding loaded = state.isEqualTo(State.LOADED); - - public enum State { - LOADING, - FAILED, - LOADED - } - - /* FXML */ @Inject - public StartController(@HealthCheckWindow Vault vault, @HealthCheckWindow Stage window, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference masterkeyRef, AtomicReference vaultConfigRef, @FxmlScene(FxmlFile.HEALTH_CHECK_LIST) Lazy checkScene, Lazy errorComponent) { - this.vault = vault; + public StartController(@HealthCheckWindow Stage window, HealthCheckComponent.LoadUnverifiedConfigResult configLoadResult, @HealthCheckWindow KeyLoadingStrategy keyLoadingStrategy, ExecutorService executor, AtomicReference masterkeyRef, AtomicReference vaultConfigRef, @FxmlScene(FxmlFile.HEALTH_CHECK_LIST) Lazy checkScene, Lazy errorComponent) { + Preconditions.checkNotNull(configLoadResult.config()); this.window = window; - this.unverifiedVaultConfig = CompletableFuture.supplyAsync(this::loadConfig, executor); + this.unverifiedVaultConfig = new SimpleObjectProperty<>(configLoadResult.config()); this.keyLoadingStrategy = keyLoadingStrategy; this.executor = executor; this.masterkeyRef = masterkeyRef; this.vaultConfigRef = vaultConfigRef; this.checkScene = checkScene; this.errorComponent = errorComponent; - this.unverifiedVaultConfig.whenCompleteAsync(this::loadedConfig, Platform::runLater); } @FXML @@ -84,29 +67,10 @@ public class StartController implements FxController { CompletableFuture.runAsync(this::loadKey, executor).whenCompleteAsync(this::loadedKey, Platform::runLater); } - private VaultConfig.UnverifiedVaultConfig loadConfig() { - assert !Platform.isFxApplicationThread(); - try { - return this.vault.getUnverifiedVaultConfig(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - private void loadedConfig(VaultConfig.UnverifiedVaultConfig cfg, Throwable exception) { - assert Platform.isFxApplicationThread(); - if (exception != null) { - state.set(State.FAILED); - } else { - assert cfg != null; - state.set(State.LOADED); - } - } - private void loadKey() { assert !Platform.isFxApplicationThread(); - assert unverifiedVaultConfig.isDone(); - var unverifiedCfg = unverifiedVaultConfig.join(); + assert unverifiedVaultConfig.get() != null; + var unverifiedCfg = unverifiedVaultConfig.get(); try (var masterkey = keyLoadingStrategy.loadKey(unverifiedCfg.getKeyId())) { var verifiedCfg = unverifiedCfg.verify(masterkey.getEncoded(), unverifiedCfg.allegedVaultVersion()); vaultConfigRef.set(verifiedCfg); @@ -150,35 +114,10 @@ public class StartController implements FxController { } } - /* Getter */ - - public BooleanBinding loadingProperty() { - return loading; - } - - public boolean isLoading() { - return loading.get(); - } - - public BooleanBinding failedProperty() { - return failed; - } - - public boolean isFailed() { - return failed.get(); - } - - public BooleanBinding loadedProperty() { - return loaded; - } - - public boolean isLoaded() { - return loaded.get(); - } - /* internal types */ private static class LoadingFailedException extends CompletionException { + LoadingFailedException(Throwable cause) { super(cause); } diff --git a/src/main/java/org/cryptomator/ui/health/StartFailController.java b/src/main/java/org/cryptomator/ui/health/StartFailController.java new file mode 100644 index 000000000..edf406213 --- /dev/null +++ b/src/main/java/org/cryptomator/ui/health/StartFailController.java @@ -0,0 +1,41 @@ +package org.cryptomator.ui.health; + +import com.google.common.base.Preconditions; +import org.cryptomator.cryptofs.VaultConfigLoadException; +import org.cryptomator.ui.common.FxController; + +import javax.inject.Inject; +import javafx.beans.binding.Bindings; +import javafx.beans.binding.BooleanBinding; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; +import javafx.fxml.FXML; +import javafx.stage.Stage; + +public class StartFailController implements FxController { + + @Inject + public StartFailController(@HealthCheckWindow Stage window, HealthCheckComponent.LoadUnverifiedConfigResult configLoadResult) { + Preconditions.checkNotNull(configLoadResult.error()); + this.window = window; + this.loadError = new SimpleObjectProperty<>(configLoadResult.error()); + this.localizedErrorMessage = new SimpleStringProperty(configLoadResult.error().getLocalizedMessage()); + this.typeParseException = Bindings.createBooleanBinding(() -> loadError.get() instanceof VaultConfigLoadException); + this.typeIOException = typeParseException.not(); + + } + + @FXML + public void close() { + window.close(); + } + + private final Stage window; + private final ObjectProperty loadError; + private final StringProperty localizedErrorMessage; + private final BooleanBinding typeIOException; + private final BooleanBinding typeParseException; + +} diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java index 0ccea096e..d1f80a40c 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/GeneralVaultOptionsController.java @@ -4,10 +4,12 @@ import org.cryptomator.common.settings.WhenUnlocked; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.controls.NumericTextField; +import org.cryptomator.ui.health.HealthCheckComponent; import javax.inject.Inject; import javafx.beans.Observable; import javafx.beans.binding.Bindings; +import javafx.event.ActionEvent; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; import javafx.scene.control.ChoiceBox; @@ -24,6 +26,7 @@ public class GeneralVaultOptionsController implements FxController { private final Stage window; private final Vault vault; + private final HealthCheckComponent.Builder healthCheckWindow; private final ResourceBundle resourceBundle; public TextField vaultName; @@ -33,9 +36,10 @@ public class GeneralVaultOptionsController implements FxController { public NumericTextField lockTimeInMinutesTextField; @Inject - GeneralVaultOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, ResourceBundle resourceBundle) { + GeneralVaultOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, HealthCheckComponent.Builder healthCheckWindow, ResourceBundle resourceBundle) { this.window = window; this.vault = vault; + this.healthCheckWindow = healthCheckWindow; this.resourceBundle = resourceBundle; } @@ -104,4 +108,8 @@ public class GeneralVaultOptionsController implements FxController { } } } + + public void startHealthCheck() { + healthCheckWindow.vault(vault).build().showHealthCheckWindow(); + } } diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/HealthVaultOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/HealthVaultOptionsController.java deleted file mode 100644 index 7b0842e97..000000000 --- a/src/main/java/org/cryptomator/ui/vaultoptions/HealthVaultOptionsController.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.cryptomator.ui.vaultoptions; - -import org.cryptomator.common.vaults.Vault; -import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.health.HealthCheckComponent; - -import javax.inject.Inject; -import javafx.event.ActionEvent; -import javafx.fxml.FXML; -import javafx.stage.Stage; - -@VaultOptionsScoped -public class HealthVaultOptionsController implements FxController { - - private final Stage window; - private final Vault vault; - private final HealthCheckComponent.Builder healthCheckWindow; - - @Inject - public HealthVaultOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, HealthCheckComponent.Builder healthCheckWindow) { - this.window = window; - this.vault = vault; - this.healthCheckWindow = healthCheckWindow; - } - - @FXML - public void startHealthCheck(ActionEvent event) { - healthCheckWindow.vault(vault).windowToClose(window).build().showHealthCheckWindow(); - } -} diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/SelectedVaultOptionsTab.java b/src/main/java/org/cryptomator/ui/vaultoptions/SelectedVaultOptionsTab.java index bfaff147a..9212b21dd 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/SelectedVaultOptionsTab.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/SelectedVaultOptionsTab.java @@ -25,9 +25,4 @@ public enum SelectedVaultOptionsTab { * Show Auto-Lock tab */ AUTOLOCK, - - /** - * Show health tab - */ - HEALTH; } diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsController.java index 662232a49..20dac7594 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsController.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsController.java @@ -24,7 +24,6 @@ public class VaultOptionsController implements FxController { public Tab mountTab; public Tab keyTab; public Tab autoLockTab; - public Tab healthTab; @Inject VaultOptionsController(@VaultOptionsWindow Stage window, ObjectProperty selectedTabProperty) { @@ -50,7 +49,6 @@ public class VaultOptionsController implements FxController { case MOUNT -> mountTab; case KEY -> keyTab; case AUTOLOCK -> autoLockTab; - case HEALTH -> healthTab; }; } diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java b/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java index 2de9421a0..cb6c109b9 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java @@ -83,9 +83,4 @@ abstract class VaultOptionsModule { @IntoMap @FxControllerKey(MasterkeyOptionsController.class) abstract FxController bindMasterkeyOptionsController(MasterkeyOptionsController controller); - - @Binds - @IntoMap - @FxControllerKey(HealthVaultOptionsController.class) - abstract FxController bindHealthOptionsController(HealthVaultOptionsController controller); } diff --git a/src/main/resources/fxml/health_start.fxml b/src/main/resources/fxml/health_start.fxml index 24b256fa8..a6b4f91b0 100644 --- a/src/main/resources/fxml/health_start.fxml +++ b/src/main/resources/fxml/health_start.fxml @@ -1,11 +1,14 @@ - - + + + + + - - - +