Jelajahi Sumber

Began error handling during add vault workflow

Sebastian Stenzel 5 tahun lalu
induk
melakukan
ac472393aa

+ 50 - 16
main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultLocationController.java

@@ -1,10 +1,13 @@
 package org.cryptomator.ui.addvaultwizard;
 
 import dagger.Lazy;
+import javafx.beans.Observable;
+import javafx.beans.binding.Bindings;
 import javafx.beans.binding.BooleanBinding;
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleBooleanProperty;
+import javafx.beans.property.SimpleStringProperty;
 import javafx.beans.property.StringProperty;
 import javafx.beans.value.ObservableValue;
 import javafx.fxml.FXML;
@@ -38,14 +41,16 @@ public class CreateNewVaultLocationController implements FxController {
 	private static final Path DEFAULT_CUSTOM_VAULT_PATH = Paths.get(System.getProperty("user.home"));
 
 	private final Stage window;
-	private final Lazy<Scene> previousScene;
-	private final Lazy<Scene> nextScene;
+	private final Lazy<Scene> chooseNameScene;
+	private final Lazy<Scene> choosePasswordScene;
 	private final LocationPresets locationPresets;
 	private final ObjectProperty<Path> vaultPath;
-	private final BooleanBinding vaultPathIsNull;
 	private final StringProperty vaultName;
 	private final ResourceBundle resourceBundle;
+	private final BooleanBinding validVaultPath;
+	private final BooleanBinding invalidVaultPath;
 	private final BooleanProperty usePresetPath;
+	private final StringProperty warningText;
 
 	private Path customVaultPath = DEFAULT_CUSTOM_VAULT_PATH;
 	public ToggleGroup predefinedLocationToggler;
@@ -53,25 +58,38 @@ public class CreateNewVaultLocationController implements FxController {
 	public RadioButton gdriveRadioButton;
 	public RadioButton customRadioButton;
 
-	//TODO: add parameter for next window
-
 	@Inject
-	CreateNewVaultLocationController(@AddVaultWizard Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_NAME) Lazy<Scene> previousScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_PASSWORD) Lazy<Scene> nextScene, LocationPresets locationPresets, ObjectProperty<Path> vaultPath, StringProperty vaultName, ResourceBundle resourceBundle) {
+	CreateNewVaultLocationController(@AddVaultWizard Stage window, @FxmlScene(FxmlFile.ADDVAULT_NEW_NAME) Lazy<Scene> chooseNameScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_PASSWORD) Lazy<Scene> choosePasswordScene, LocationPresets locationPresets, ObjectProperty<Path> vaultPath, StringProperty vaultName, ResourceBundle resourceBundle) {
 		this.window = window;
-		this.previousScene = previousScene;
-		this.nextScene = nextScene;
+		this.chooseNameScene = chooseNameScene;
+		this.choosePasswordScene = choosePasswordScene;
 		this.locationPresets = locationPresets;
 		this.vaultPath = vaultPath;
 		this.vaultName = vaultName;
 		this.resourceBundle = resourceBundle;
-		this.vaultPathIsNull = vaultPath.isNull();
+		this.validVaultPath = Bindings.createBooleanBinding(this::isValidVaultPath, vaultPath);
+		this.invalidVaultPath = validVaultPath.not();
 		this.usePresetPath = new SimpleBooleanProperty();
+		this.warningText = new SimpleStringProperty();
+	}
+
+	private boolean isValidVaultPath() {
+		return vaultPath.get() != null && Files.notExists(vaultPath.get());
 	}
 
 	@FXML
 	public void initialize() {
 		predefinedLocationToggler.selectedToggleProperty().addListener(this::togglePredefinedLocation);
 		usePresetPath.bind(predefinedLocationToggler.selectedToggleProperty().isNotEqualTo(customRadioButton));
+		vaultPath.addListener(this::vaultPathDidChange);
+	}
+
+	private void vaultPathDidChange(@SuppressWarnings("unused") ObservableValue<? extends Path> observable, @SuppressWarnings("unused") Path oldValue, Path newValue) {
+		if (!Files.notExists(newValue)) {
+			warningText.set(resourceBundle.getString("addvaultwizard.new.fileAlreadyExists"));
+		} else {
+			warningText.set(null);
+		}
 	}
 
 	private void togglePredefinedLocation(@SuppressWarnings("unused") ObservableValue<? extends Toggle> observable, @SuppressWarnings("unused") Toggle oldValue, Toggle newValue) {
@@ -86,7 +104,7 @@ public class CreateNewVaultLocationController implements FxController {
 
 	@FXML
 	public void back() {
-		window.setScene(previousScene.get());
+		window.setScene(chooseNameScene.get());
 	}
 
 	@FXML
@@ -96,10 +114,10 @@ public class CreateNewVaultLocationController implements FxController {
 			assert Files.isDirectory(vaultPath.get().getParent());
 			Path createdDir = Files.createDirectory(vaultPath.get());
 			Files.delete(createdDir); // assert: dir exists and is empty
-			window.setScene(nextScene.get());
+			window.setScene(choosePasswordScene.get());
 		} catch (FileAlreadyExistsException e) {
 			LOG.warn("Can not use already existing vault path: {}", vaultPath.get());
-			// TODO show specific error text "vault can not be created at this path because some object already exists"
+			warningText.set(resourceBundle.getString("addvaultwizard.new.fileAlreadyExists"));
 		} catch (NoSuchFileException | DirectoryNotEmptyException e) {
 			LOG.error("Failed to delete recently created directory.", e);
 			// TODO show generic error text for unexpected exception
@@ -131,12 +149,12 @@ public class CreateNewVaultLocationController implements FxController {
 		return vaultPath;
 	}
 
-	public boolean isVaultPathIsNull() {
-		return vaultPathIsNull.get();
+	public BooleanBinding invalidVaultPathProperty() {
+		return invalidVaultPath;
 	}
 
-	public BooleanBinding vaultPathIsNullProperty() {
-		return vaultPathIsNull;
+	public Boolean getInvalidVaultPath() {
+		return invalidVaultPath.get();
 	}
 
 	public LocationPresets getLocationPresets() {
@@ -150,4 +168,20 @@ public class CreateNewVaultLocationController implements FxController {
 	public boolean getUsePresetPath() {
 		return usePresetPath.get();
 	}
+
+	public StringProperty warningTextProperty() {
+		return warningText;
+	}
+
+	public String getWarningText() {
+		return warningText.get();
+	}
+
+	public BooleanBinding showWarningProperty() {
+		return warningText.isNotEmpty();
+	}
+
+	public boolean isShowWarning() {
+		return showWarningProperty().get();
+	}
 }

+ 55 - 36
main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultNameController.java

@@ -1,7 +1,14 @@
 package org.cryptomator.ui.addvaultwizard;
 
 import dagger.Lazy;
+import javafx.beans.Observable;
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.BooleanBinding;
+import javafx.beans.binding.StringBinding;
+import javafx.beans.property.ObjectProperty;
+import javafx.beans.property.SimpleStringProperty;
 import javafx.beans.property.StringProperty;
+import javafx.beans.value.ObservableValue;
 import javafx.fxml.FXML;
 import javafx.scene.Scene;
 import javafx.scene.control.TextField;
@@ -16,32 +23,52 @@ import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ResourceBundle;
+import java.util.regex.Pattern;
 
-/**
- * TODO: Add trim() filter to vaultName
- */
 @AddVaultWizardScoped
 public class CreateNewVaultNameController implements FxController {
 
+	private static final Pattern VALID_NAME_PATTERN = Pattern.compile("[\\w -]+", Pattern.UNICODE_CHARACTER_CLASS);
+
 	public TextField textField;
 	private final Stage window;
 	private final Lazy<Scene> welcomeScene;
-	private final Lazy<Scene> nextScene;
+	private final Lazy<Scene> chooseLocationScene;
+	private final ObjectProperty<Path> vaultPath;
 	private final StringProperty vaultName;
-	private final ResourceBundle resourceBundle;
+	private final BooleanBinding validVaultName;
+	private final BooleanBinding invalidVaultName;
+	private final StringBinding warningText;
 
 	@Inject
-	CreateNewVaultNameController(@AddVaultWizard Stage window, @FxmlScene(FxmlFile.ADDVAULT_WELCOME) Lazy<Scene> welcomeScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy<Scene> nextScene, StringProperty vaultName, ResourceBundle resourceBundle) {
+	CreateNewVaultNameController(@AddVaultWizard Stage window, @FxmlScene(FxmlFile.ADDVAULT_WELCOME) Lazy<Scene> welcomeScene, @FxmlScene(FxmlFile.ADDVAULT_NEW_LOCATION) Lazy<Scene> chooseLocationScene, ObjectProperty<Path> vaultPath, StringProperty vaultName, ResourceBundle resourceBundle) {
 		this.window = window;
 		this.welcomeScene = welcomeScene;
-		this.nextScene = nextScene;
+		this.chooseLocationScene = chooseLocationScene;
+		this.vaultPath = vaultPath;
 		this.vaultName = vaultName;
-		this.resourceBundle = resourceBundle;
+		this.validVaultName = Bindings.createBooleanBinding(this::isValidVaultName, vaultName);
+		this.invalidVaultName = validVaultName.not();
+		this.warningText = Bindings.when(vaultName.isNotEmpty().and(invalidVaultName)).then(resourceBundle.getString("addvaultwizard.new.invalidName")).otherwise((String) null);
 	}
 
 	@FXML
 	public void initialize() {
 		vaultName.bind(textField.textProperty());
+		vaultName.addListener(this::vaultNameChanged);
+	}
+
+	public boolean isValidVaultName() {
+		return vaultName.get() != null && VALID_NAME_PATTERN.matcher(vaultName.get().trim()).matches();
+	}
+
+	private void vaultNameChanged(@SuppressWarnings("unused") Observable observable) {
+		if (isValidVaultName()) {
+			if (vaultPath.get() != null) {
+				// update vaultPath if it is already set but the user went back to change its name:
+				vaultPath.set(vaultPath.get().resolveSibling(vaultName.get()));
+			}
+		}
 	}
 
 	@FXML
@@ -51,41 +78,33 @@ public class CreateNewVaultNameController implements FxController {
 
 	@FXML
 	public void next() {
-		if (nameIsValid()) {
-			window.setScene(nextScene.get());
-		} else {
-			//TODO
-		}
+		window.setScene(chooseLocationScene.get());
 	}
 
-	/**
-	 * Checks if {@link CreateNewVaultNameController#vaultName}is a valid directory name in the OS by creating and deleting a directory with the given name in the temporary section of the OS
-	 * TODO: Logging
-	 *
-	 * @return true, if a directory with the name already exists or can be created
-	 */
-	private boolean nameIsValid() {
-		try {
-			Path tmp = Files.createTempDirectory(vaultName.get());
-			Files.deleteIfExists(tmp.toAbsolutePath());
-			return true;
-		} catch (FileAlreadyExistsException e) {
-			return true;
-		} catch (IOException e) {
-			return false;
-		} catch (IllegalArgumentException e) {
-			return false;
-		}
+	/* Getter/Setter */
+
+	public BooleanBinding invalidVaultNameProperty() {
+		return invalidVaultName;
 	}
 
-	/* Getter/Setter */
+	public boolean isInvalidVaultName() {
+		return invalidVaultName.get();
+	}
+
+	public StringBinding warningTextProperty() {
+		return warningText;
+	}
+
+	public String getWarningText() {
+		return warningText.get();
+	}
 
-	public String getVaultName() {
-		return vaultName.get();
+	public BooleanBinding showWarningProperty() {
+		return warningText.isNotEmpty();
 	}
 
-	public StringProperty vaultNameProperty() {
-		return vaultName;
+	public boolean isShowWarning() {
+		return showWarningProperty().get();
 	}
 
 }

+ 16 - 4
main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/CreateNewVaultPasswordController.java

@@ -26,9 +26,12 @@ import org.cryptomator.ui.controls.FontAwesome5IconView;
 import org.cryptomator.ui.controls.SecPasswordField;
 import org.cryptomator.ui.util.PasswordStrengthUtil;
 import org.fxmisc.easybind.EasyBind;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ResourceBundle;
@@ -36,6 +39,8 @@ import java.util.ResourceBundle;
 @AddVaultWizardScoped
 public class CreateNewVaultPasswordController implements FxController {
 
+	private static final Logger LOG = LoggerFactory.getLogger(CreateNewVaultPasswordController.class);
+
 	private final Stage window;
 	private final Lazy<Scene> previousScene;
 	private final StringProperty vaultName;
@@ -108,17 +113,24 @@ public class CreateNewVaultPasswordController implements FxController {
 	@FXML
 	public void finish() {
 		VaultSettings vaultSettings = VaultSettings.withRandomId();
-		vaultSettings.path().setValue(vaultPath.get().resolve(vaultName.get()));
+		vaultSettings.path().setValue(vaultPath.get());
 		Vault newVault = vaultFactory.get(vaultSettings);
 		try {
-			//TODO: why is creating the directory not part of the creation process?
 			Files.createDirectory(vaultSettings.path().get());
+		} catch (FileAlreadyExistsException e) {
+			// TODO show specific error screen
+			LOG.error("", e);
+		} catch (IOException e) {
+			// TODO show generic error screen
+			LOG.error("", e);
+		}
+		try {
 			newVault.create(passwordField.getCharacters());
 			vaults.add(newVault);
 			window.close();
 		} catch (IOException e) {
-			e.printStackTrace();
-			//TODO
+			// TODO show generic error screen
+			LOG.error("", e);
 		}
 	}
 

+ 7 - 1
main/ui/src/main/resources/fxml/addvault_new_location.fxml

@@ -11,6 +11,8 @@
 <?import javafx.scene.layout.Region?>
 <?import javafx.scene.layout.VBox?>
 <?import org.cryptomator.ui.controls.FontAwesome5IconView?>
+<?import javafx.scene.text.TextFlow?>
+<?import javafx.scene.text.Text?>
 <VBox xmlns="http://javafx.com/javafx"
 	  xmlns:fx="http://javafx.com/fxml"
 	  fx:controller="org.cryptomator.ui.addvaultwizard.CreateNewVaultLocationController"
@@ -46,13 +48,17 @@
 
 		<Label text="%addvaultwizard.new.locationLabel" labelFor="$locationTextField"/>
 		<TextField fx:id="locationTextField" promptText="%addvaultwizard.new.locationPrompt" text="${controller.vaultPath}" disable="true" HBox.hgrow="ALWAYS"/>
+		<TextFlow styleClass="text-flow" visible="${controller.showWarning}" prefHeight="22">
+			<FontAwesome5IconView glyph="EXCLAMATION_TRIANGLE"/>
+			<Text text="${controller.warningText}"/>
+		</TextFlow>
 
 		<Region VBox.vgrow="ALWAYS"/>
 
 		<ButtonBar buttonMinWidth="120" buttonOrder="B+X">
 			<buttons>
 				<Button text="%generic.button.back" ButtonBar.buttonData="BACK_PREVIOUS" onAction="#back"/>
-				<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" onAction="#next" defaultButton="true" disable="${controller.vaultPathIsNull}"/>
+				<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" onAction="#next" defaultButton="true" disable="${controller.invalidVaultPath}"/>
 			</buttons>
 		</ButtonBar>
 	</children>

+ 8 - 1
main/ui/src/main/resources/fxml/addvault_new_name.fxml

@@ -8,6 +8,9 @@
 <?import javafx.scene.layout.HBox?>
 <?import javafx.scene.layout.Region?>
 <?import javafx.scene.layout.VBox?>
+<?import javafx.scene.text.TextFlow?>
+<?import javafx.scene.text.Text?>
+<?import org.cryptomator.ui.controls.FontAwesome5IconView?>
 <VBox xmlns="http://javafx.com/javafx"
 	  xmlns:fx="http://javafx.com/fxml"
 	  fx:controller="org.cryptomator.ui.addvaultwizard.CreateNewVaultNameController"
@@ -23,13 +26,17 @@
 
 		<Label text="%addvaultwizard.new.nameInstruction" labelFor="$textField"/>
 		<TextField fx:id="textField" promptText="%addvaultwizard.new.namePrompt" HBox.hgrow="ALWAYS"/>
+		<TextFlow styleClass="text-flow" visible="${controller.showWarning}" prefHeight="22">
+			<FontAwesome5IconView glyph="EXCLAMATION_TRIANGLE"/>
+			<Text text="${controller.warningText}"/>
+		</TextFlow>
 
 		<Region VBox.vgrow="ALWAYS"/>
 
 		<ButtonBar buttonMinWidth="120" buttonOrder="B+X">
 			<buttons>
 				<Button text="%generic.button.back" ButtonBar.buttonData="BACK_PREVIOUS" onAction="#back"/>
-				<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" onAction="#next" defaultButton="true" disable="${controller.vaultName.empty}"/>
+				<Button text="%generic.button.next" ButtonBar.buttonData="NEXT_FORWARD" onAction="#next" defaultButton="true" disable="${controller.invalidVaultName}"/>
 			</buttons>
 		</ButtonBar>
 	</children>

+ 2 - 0
main/ui/src/main/resources/i18n/strings.properties

@@ -21,6 +21,8 @@ addvaultwizard.new.directoryPickerLabel=Custom Location
 addvaultwizard.new.directoryPickerButton=Choose…
 addvaultwizard.new.directoryPickerTitle=Select Directory
 addvaultwizard.new.enterPassword=Please enter a password for your vault:
+addvaultwizard.new.fileAlreadyExists=Vault can not be created at this path because some object already exists.
+addvaultwizard.new.invalidName=Invalid vault name. Please consider a regular directory name
 addvaultwizard.new.reenterPassword=Please confirm the password:
 addvaultwizard.new.passwordsMatch=Passwords match!
 addvaultwizard.new.passwordsDoNotMatch=Passwords do not match