Browse Source

Refactor lock/unlock workflows:
* don't set vault state on successful lock workflow
* improved error handling

Armin Schrenk 4 years ago
parent
commit
cd5c55aad7

+ 13 - 10
main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java

@@ -4,6 +4,7 @@ import dagger.Lazy;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultState;
 import org.cryptomator.common.vaults.Volume;
+import org.cryptomator.ui.common.ErrorComponent;
 import org.cryptomator.ui.common.FxmlFile;
 import org.cryptomator.ui.common.FxmlScene;
 import org.cryptomator.ui.common.UserInteractionLock;
@@ -35,14 +36,16 @@ public class LockWorkflow extends Task<Void> {
 	private final UserInteractionLock<LockModule.ForceLockDecision> forceLockDecisionLock;
 	private final Lazy<Scene> lockForcedScene;
 	private final Lazy<Scene> lockFailedScene;
+	private final ErrorComponent.Builder errorComponent;
 
 	@Inject
-	public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock<LockModule.ForceLockDecision> forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy<Scene> lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy<Scene> lockFailedScene) {
+	public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock<LockModule.ForceLockDecision> forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy<Scene> lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy<Scene> lockFailedScene, ErrorComponent.Builder errorComponent) {
 		this.lockWindow = lockWindow;
 		this.vault = vault;
 		this.forceLockDecisionLock = forceLockDecisionLock;
 		this.lockForcedScene = lockForcedScene;
 		this.lockFailedScene = lockFailedScene;
+		this.errorComponent = errorComponent;
 	}
 
 	@Override
@@ -77,23 +80,23 @@ public class LockWorkflow extends Task<Void> {
 		return forceLockDecisionLock.awaitInteraction();
 	}
 
-	@Override
-	protected void scheduled() {
-		vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
-	}
-
 	@Override
 	protected void succeeded() {
 		LOG.info("Lock of {} succeeded.", vault.getDisplayName());
-		vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED);
+		//DO NOT SET VAULT STATE HERE, this is done by the vault internally
 	}
 
 	@Override
 	protected void failed() {
-		LOG.warn("Failed to lock {}.", vault.getDisplayName());
+		final var throwable = super.getException();
+		LOG.warn("Lock of {} failed.", vault.getDisplayName(), throwable);
 		vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED);
-		lockWindow.setScene(lockFailedScene.get());
-		lockWindow.show();
+		if (throwable instanceof Volume.VolumeException) {
+			lockWindow.setScene(lockFailedScene.get());
+			lockWindow.show();
+		} else {
+			errorComponent.cause(throwable).window(lockWindow).build().showErrorScene();
+		}
 	}
 
 	@Override

+ 26 - 34
main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java

@@ -73,22 +73,15 @@ public class UnlockWorkflow extends Task<Boolean> {
 		this.successScene = successScene;
 		this.invalidMountPointScene = invalidMountPointScene;
 		this.errorComponent = errorComponent;
-
-		setOnFailed(event -> {
-			Throwable throwable = event.getSource().getException();
-			if (throwable instanceof InvalidMountPointException e) {
-				handleInvalidMountPoint(e);
-			} else {
-				handleGenericError(throwable);
-			}
-		});
 	}
 
 	@Override
 	protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException {
 		try {
 			if (attemptUnlock()) {
-				handleSuccess();
+				if (savePassword.get()) {
+					savePasswordToSystemkeychain(); //savePassword will be wiped on method return, so it must be set here
+				}
 				return true;
 			} else {
 				cancel(false); // set Tasks state to cancelled
@@ -131,24 +124,6 @@ public class UnlockWorkflow extends Task<Boolean> {
 		return passwordEntryLock.awaitInteraction();
 	}
 
-	private void handleSuccess() {
-		LOG.info("Unlock of '{}' succeeded.", vault.getDisplayName());
-		if (savePassword.get()) {
-			savePasswordToSystemkeychain();
-		}
-		switch (vault.getVaultSettings().actionAfterUnlock().get()) {
-			case ASK -> Platform.runLater(() -> {
-				window.setScene(successScene.get());
-				window.show();
-			});
-			case REVEAL -> {
-				Platform.runLater(window::close);
-				vaultService.reveal(vault);
-			}
-			case IGNORE -> Platform.runLater(window::close);
-		}
-	}
-
 	private void savePasswordToSystemkeychain() {
 		if (keychain.isSupported()) {
 			try {
@@ -193,7 +168,7 @@ public class UnlockWorkflow extends Task<Boolean> {
 
 	private void handleGenericError(Throwable e) {
 		LOG.error("Unlock failed for technical reasons.", e);
-		errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene();
+		errorComponent.cause(e).window(window).build().showErrorScene();
 	}
 
 	private void wipePassword(char[] pw) {
@@ -202,23 +177,40 @@ public class UnlockWorkflow extends Task<Boolean> {
 		}
 	}
 
-	@Override
-	protected void scheduled() {
-		vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING);
-	}
-
 	@Override
 	protected void succeeded() {
+		LOG.info("Unlock of '{}' succeeded.", vault.getDisplayName());
+
+		switch (vault.getVaultSettings().actionAfterUnlock().get()) {
+			case ASK -> Platform.runLater(() -> {
+				window.setScene(successScene.get());
+				window.show();
+			});
+			case REVEAL -> {
+				Platform.runLater(window::close);
+				vaultService.reveal(vault);
+			}
+			case IGNORE -> Platform.runLater(window::close);
+		}
+
 		vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED);
 	}
 
 	@Override
 	protected void failed() {
+		LOG.info("Unlock of '{}' failed.", vault.getDisplayName());
+		Throwable throwable = super.getException();
+		if (throwable instanceof InvalidMountPointException e) {
+			handleInvalidMountPoint(e);
+		} else {
+			handleGenericError(throwable);
+		}
 		vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED);
 	}
 
 	@Override
 	protected void cancelled() {
+		LOG.debug("Unlock of '{}' canceled.", vault.getDisplayName());
 		vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED);
 	}