From 5143fdccbb5a50a0f237019540e8ef99c1274b55 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 11 Nov 2022 19:42:59 +0100 Subject: [PATCH] Refactor mount point setting and controller : * remove CustomMountPath, winDriveLetter and usesCustomMountPath * new property mountPoint (can be null) * differentiation between driveLetter and directory happens in controller --- .../common/mount/WindowsDriveLetters.java | 49 ++---- .../common/settings/VaultSettings.java | 44 +---- .../settings/VaultSettingsJsonAdapter.java | 37 +++-- .../org/cryptomator/common/vaults/Vault.java | 7 +- .../vaultoptions/MountOptionsController.java | 150 +++++++++++------- .../resources/fxml/vault_options_mount.fxml | 8 +- src/main/resources/i18n/strings.properties | 4 +- .../VaultSettingsJsonAdapterTest.java | 2 - 8 files changed, 154 insertions(+), 147 deletions(-) diff --git a/src/main/java/org/cryptomator/common/mount/WindowsDriveLetters.java b/src/main/java/org/cryptomator/common/mount/WindowsDriveLetters.java index 6f56425e8..ad65ff8b9 100644 --- a/src/main/java/org/cryptomator/common/mount/WindowsDriveLetters.java +++ b/src/main/java/org/cryptomator/common/mount/WindowsDriveLetters.java @@ -5,7 +5,6 @@ *******************************************************************************/ package org.cryptomator.common.mount; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.apache.commons.lang3.SystemUtils; @@ -13,8 +12,10 @@ import javax.inject.Inject; import javax.inject.Singleton; import java.nio.file.FileSystems; import java.nio.file.Path; +import java.util.Collections; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.StreamSupport; @@ -22,64 +23,48 @@ import java.util.stream.StreamSupport; @Singleton public final class WindowsDriveLetters { - private static final Set A_TO_Z; + private static final Set A_TO_Z; static { - try (IntStream stream = IntStream.rangeClosed('A', 'Z')) { - A_TO_Z = stream.mapToObj(i -> String.valueOf((char) i)).collect(ImmutableSet.toImmutableSet()); - } + var sortedSet = new TreeSet(); + IntStream.rangeClosed('A', 'Z').mapToObj(i -> Path.of((char) i + ":\\")).forEach(sortedSet::add); + A_TO_Z = Collections.unmodifiableSet(sortedSet); } @Inject public WindowsDriveLetters() { } - public Set getAllDriveLetters() { + public Set getAll() { return A_TO_Z; } - public Set getOccupiedDriveLetters() { + public Set getOccupied() { if (!SystemUtils.IS_OS_WINDOWS) { return Set.of(); } else { Iterable rootDirs = FileSystems.getDefault().getRootDirectories(); - return StreamSupport.stream(rootDirs.spliterator(), false).map(p -> p.toString().substring(0, 1)).collect(Collectors.toSet()); + return StreamSupport.stream(rootDirs.spliterator(), false).collect(Collectors.toUnmodifiableSet()); } } - public Set getAvailableDriveLetters() { - return Sets.difference(getAllDriveLetters(), getOccupiedDriveLetters()); - } - - public Optional getAvailableDriveLetter() { - return getAvailableDriveLetters().stream().findFirst(); - } - - public Optional getAvailableDriveLetterPath() { - return getAvailableDriveLetter().map(this::toPath); + public Set getAvailable() { + return Sets.difference(getAll(), getOccupied()); } /** - * Skips A and B and only returns them if all other are occupied. + * Skips A and B and only returns them if all others are occupied. * * @return an Optional containing either the letter of a free drive letter or empty, if none is available */ - public Optional getDesiredAvailableDriveLetter() { - var availableDriveLetters = getAvailableDriveLetters(); - var optString = availableDriveLetters.stream().filter(s -> !(s.equals("A") || s.equals("B"))).findFirst(); + public Optional getFirstDesiredAvailable() { + var availableDriveLetters = getAvailable(); + var optString = availableDriveLetters.stream().filter(this::notAOrB).findFirst(); return optString.or(() -> availableDriveLetters.stream().findFirst()); } - /** - * Skips A and B and only returns them if all other are occupied. - * - * @return an Optional containing either the path to a free drive letter or empty, if none is available - */ - public Optional getDesiredAvailableDriveLetterPath() { - return getDesiredAvailableDriveLetter().map(this::toPath); + private boolean notAOrB(Path driveLetter) { + return !(Path.of("A:\\").equals(driveLetter) || Path.of("B:\\").equals(driveLetter)); } - public Path toPath(String driveLetter) { - return Path.of(driveLetter + ":\\"); - } } diff --git a/src/main/java/org/cryptomator/common/settings/VaultSettings.java b/src/main/java/org/cryptomator/common/settings/VaultSettings.java index fb4e5357c..57c114718 100644 --- a/src/main/java/org/cryptomator/common/settings/VaultSettings.java +++ b/src/main/java/org/cryptomator/common/settings/VaultSettings.java @@ -6,7 +6,6 @@ package org.cryptomator.common.settings; import com.google.common.base.CharMatcher; -import com.google.common.base.Strings; import com.google.common.io.BaseEncoding; import javafx.beans.Observable; @@ -22,7 +21,6 @@ import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import java.nio.file.Path; import java.util.Objects; -import java.util.Optional; import java.util.Random; /** @@ -32,7 +30,6 @@ public class VaultSettings { public static final boolean DEFAULT_UNLOCK_AFTER_STARTUP = false; public static final boolean DEFAULT_REVEAL_AFTER_MOUNT = true; - public static final boolean DEFAULT_USES_INDIVIDUAL_MOUNTPATH = false; public static final boolean DEFAULT_USES_READONLY_MODE = false; public static final String DEFAULT_MOUNT_FLAGS = ""; public static final int DEFAULT_MAX_CLEARTEXT_FILENAME_LENGTH = -1; @@ -45,11 +42,8 @@ public class VaultSettings { private final String id; private final ObjectProperty path = new SimpleObjectProperty<>(); private final StringProperty displayName = new SimpleStringProperty(); - private final StringProperty winDriveLetter = new SimpleStringProperty(); private final BooleanProperty unlockAfterStartup = new SimpleBooleanProperty(DEFAULT_UNLOCK_AFTER_STARTUP); private final BooleanProperty revealAfterMount = new SimpleBooleanProperty(DEFAULT_REVEAL_AFTER_MOUNT); - private final BooleanProperty useCustomMountPath = new SimpleBooleanProperty(DEFAULT_USES_INDIVIDUAL_MOUNTPATH); - private final StringProperty customMountPath = new SimpleStringProperty(); private final BooleanProperty usesReadOnlyMode = new SimpleBooleanProperty(DEFAULT_USES_READONLY_MODE); private final StringProperty mountFlags = new SimpleStringProperty(DEFAULT_MOUNT_FLAGS); private final IntegerProperty maxCleartextFilenameLength = new SimpleIntegerProperty(DEFAULT_MAX_CLEARTEXT_FILENAME_LENGTH); @@ -73,7 +67,7 @@ public class VaultSettings { } Observable[] observables() { - return new Observable[]{path, displayName, winDriveLetter, unlockAfterStartup, revealAfterMount, useCustomMountPath, customMountPath, usesReadOnlyMode, mountFlags, maxCleartextFilenameLength, actionAfterUnlock, autoLockWhenIdle, autoLockIdleSeconds}; + return new Observable[]{actionAfterUnlock, autoLockIdleSeconds, autoLockWhenIdle, displayName, maxCleartextFilenameLength, mountFlags, mountPoint, path, revealAfterMount, unlockAfterStartup, usesReadOnlyMode}; } public static VaultSettings withRandomId() { @@ -117,18 +111,6 @@ public class VaultSettings { return mountName; } - public StringProperty winDriveLetter() { - return winDriveLetter; - } - - public Optional getWinDriveLetter() { - String current = this.winDriveLetter.get(); - if (!Strings.isNullOrEmpty(current)) { - return Optional.of(current); - } - return Optional.empty(); - } - public BooleanProperty unlockAfterStartup() { return unlockAfterStartup; } @@ -137,20 +119,12 @@ public class VaultSettings { return revealAfterMount; } - public BooleanProperty useCustomMountPath() { - return useCustomMountPath; + public Path getMountPoint() { + return mountPoint.get(); } - public StringProperty customMountPath() { - return customMountPath; - } - - public Optional getCustomMountPath() { - if (useCustomMountPath.get()) { - return Optional.ofNullable(Strings.emptyToNull(customMountPath.get())); - } else { - return Optional.empty(); - } + public ObjectProperty mountPoint() { + return mountPoint; } public BooleanProperty usesReadOnlyMode() { @@ -196,12 +170,4 @@ public class VaultSettings { return false; } } - - public Path getMountPoint() { - return mountPoint.get(); - } - - public ObjectProperty mountPointProperty() { - return mountPoint; - } } diff --git a/src/main/java/org/cryptomator/common/settings/VaultSettingsJsonAdapter.java b/src/main/java/org/cryptomator/common/settings/VaultSettingsJsonAdapter.java index a3a3118dc..0f1aa7edd 100644 --- a/src/main/java/org/cryptomator/common/settings/VaultSettingsJsonAdapter.java +++ b/src/main/java/org/cryptomator/common/settings/VaultSettingsJsonAdapter.java @@ -6,11 +6,14 @@ package org.cryptomator.common.settings; import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; import java.nio.file.Paths; class VaultSettingsJsonAdapter { @@ -22,11 +25,10 @@ class VaultSettingsJsonAdapter { out.name("id").value(value.getId()); out.name("path").value(value.path().get().toString()); out.name("displayName").value(value.displayName().get()); - out.name("winDriveLetter").value(value.winDriveLetter().get()); out.name("unlockAfterStartup").value(value.unlockAfterStartup().get()); out.name("revealAfterMount").value(value.revealAfterMount().get()); - out.name("useCustomMountPath").value(value.useCustomMountPath().get()); - out.name("customMountPath").value(value.customMountPath().get()); + var mountPoint = value.mountPoint().get(); + out.name("mountPoint").value(mountPoint != null ? mountPoint.toAbsolutePath().toString() : null); out.name("usesReadOnlyMode").value(value.usesReadOnlyMode().get()); out.name("mountFlags").value(value.mountFlags().get()); out.name("maxCleartextFilenameLength").value(value.maxCleartextFilenameLength().get()); @@ -36,18 +38,18 @@ class VaultSettingsJsonAdapter { out.endObject(); } + //TODO: usesCustomMountPath, customMountPath and winDriveLetter removed + // -> migration required public VaultSettings read(JsonReader in) throws IOException { String id = null; String path = null; String mountName = null; //see https://github.com/cryptomator/cryptomator/pull/1318 String displayName = null; - String customMountPath = null; - String winDriveLetter = null; boolean unlockAfterStartup = VaultSettings.DEFAULT_UNLOCK_AFTER_STARTUP; boolean revealAfterMount = VaultSettings.DEFAULT_REVEAL_AFTER_MOUNT; - boolean useCustomMountPath = VaultSettings.DEFAULT_USES_INDIVIDUAL_MOUNTPATH; boolean usesReadOnlyMode = VaultSettings.DEFAULT_USES_READONLY_MODE; String mountFlags = VaultSettings.DEFAULT_MOUNT_FLAGS; + Path mountPoint = null; int maxCleartextFilenameLength = VaultSettings.DEFAULT_MAX_CLEARTEXT_FILENAME_LENGTH; WhenUnlocked actionAfterUnlock = VaultSettings.DEFAULT_ACTION_AFTER_UNLOCK; boolean autoLockWhenIdle = VaultSettings.DEFAULT_AUTOLOCK_WHEN_IDLE; @@ -61,13 +63,17 @@ class VaultSettingsJsonAdapter { case "path" -> path = in.nextString(); case "mountName" -> mountName = in.nextString(); //see https://github.com/cryptomator/cryptomator/pull/1318 case "displayName" -> displayName = in.nextString(); - case "winDriveLetter" -> winDriveLetter = in.nextString(); case "unlockAfterStartup" -> unlockAfterStartup = in.nextBoolean(); case "revealAfterMount" -> revealAfterMount = in.nextBoolean(); - case "usesIndividualMountPath", "useCustomMountPath" -> useCustomMountPath = in.nextBoolean(); - case "individualMountPath", "customMountPath" -> customMountPath = in.nextString(); case "usesReadOnlyMode" -> usesReadOnlyMode = in.nextBoolean(); case "mountFlags" -> mountFlags = in.nextString(); + case "mountPoint" -> { + if (JsonToken.NULL == in.peek()) { + in.nextNull(); + } else { + mountPoint = parseMountPoint(in.nextString()); + } + } case "maxCleartextFilenameLength" -> maxCleartextFilenameLength = in.nextInt(); case "actionAfterUnlock" -> actionAfterUnlock = parseActionAfterUnlock(in.nextString()); case "autoLockWhenIdle" -> autoLockWhenIdle = in.nextBoolean(); @@ -87,20 +93,27 @@ class VaultSettingsJsonAdapter { vaultSettings.displayName().set(mountName); } vaultSettings.path().set(Paths.get(path)); - vaultSettings.winDriveLetter().set(winDriveLetter); vaultSettings.unlockAfterStartup().set(unlockAfterStartup); vaultSettings.revealAfterMount().set(revealAfterMount); - vaultSettings.useCustomMountPath().set(useCustomMountPath); - vaultSettings.customMountPath().set(customMountPath); vaultSettings.usesReadOnlyMode().set(usesReadOnlyMode); vaultSettings.mountFlags().set(mountFlags); vaultSettings.maxCleartextFilenameLength().set(maxCleartextFilenameLength); vaultSettings.actionAfterUnlock().set(actionAfterUnlock); vaultSettings.autoLockWhenIdle().set(autoLockWhenIdle); vaultSettings.autoLockIdleSeconds().set(autoLockIdleSeconds); + vaultSettings.mountPoint().set(mountPoint); return vaultSettings; } + private Path parseMountPoint(String mountPoint) { + try { + return Path.of(mountPoint); + } catch (InvalidPathException e) { + LOG.warn("Invalid string as mount point. Defaulting to null."); + return null; + } + } + private WhenUnlocked parseActionAfterUnlock(String actionAfterUnlockName) { try { return WhenUnlocked.valueOf(actionAfterUnlockName.toUpperCase()); diff --git a/src/main/java/org/cryptomator/common/vaults/Vault.java b/src/main/java/org/cryptomator/common/vaults/Vault.java index 95e08eaa0..835560120 100644 --- a/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -12,6 +12,7 @@ import com.google.common.base.Strings; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Constants; import org.cryptomator.common.Environment; +import org.cryptomator.common.mount.WindowsDriveLetters; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.cryptofs.CryptoFileSystem; @@ -83,12 +84,13 @@ public class Vault { private final BooleanBinding needsMigration; private final BooleanBinding unknownError; private final ObjectBinding mountPoint; + private final WindowsDriveLetters windowsDriveLetters; private final BooleanProperty showingStats; private AtomicReference mountHandle = new AtomicReference<>(null); @Inject - Vault(Environment env, Settings settings, VaultSettings vaultSettings, VaultConfigCache configCache, AtomicReference cryptoFileSystem, VaultState state, @Named("lastKnownException") ObjectProperty lastKnownException, ObservableValue mountService, VaultStats stats) { + Vault(Environment env, Settings settings, VaultSettings vaultSettings, VaultConfigCache configCache, AtomicReference cryptoFileSystem, VaultState state, @Named("lastKnownException") ObjectProperty lastKnownException, ObservableValue mountService, VaultStats stats, WindowsDriveLetters windowsDriveLetters) { this.env = env; this.settings = settings; this.vaultSettings = vaultSettings; @@ -107,6 +109,7 @@ public class Vault { this.needsMigration = Bindings.createBooleanBinding(this::isNeedsMigration, state); this.unknownError = Bindings.createBooleanBinding(this::isUnknownError, state); this.mountPoint = Bindings.createObjectBinding(this::getMountPoint, state); + this.windowsDriveLetters = windowsDriveLetters; this.showingStats = new SimpleBooleanProperty(false); } @@ -178,7 +181,7 @@ public class Vault { if (mountProvider.hasCapability(MOUNT_TO_SYSTEM_CHOSEN_PATH)) { // no need to set a mount point } else if (mountProvider.hasCapability(MOUNT_AS_DRIVE_LETTER)) { - // TODO find any free drive letter? + builder.setMountpoint(windowsDriveLetters.getFirstDesiredAvailable().orElseThrow()); } else if (mountProvider.hasCapability(MOUNT_WITHIN_EXISTING_PARENT)) { Files.createDirectories(defaultMountPointBase); builder.setMountpoint(defaultMountPointBase); diff --git a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java index ffb003d4a..8c28b1a5d 100644 --- a/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java +++ b/src/main/java/org/cryptomator/ui/vaultoptions/MountOptionsController.java @@ -1,15 +1,13 @@ package org.cryptomator.ui.vaultoptions; -import com.google.common.base.Strings; import org.cryptomator.common.Environment; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.mount.WindowsDriveLetters; +import org.cryptomator.common.vaults.Vault; import org.cryptomator.integrations.mount.MountCapability; import org.cryptomator.integrations.mount.MountService; import org.cryptomator.ui.common.FxController; import javax.inject.Inject; -import javafx.beans.binding.Bindings; import javafx.beans.value.ObservableValue; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; @@ -38,10 +36,13 @@ public class MountOptionsController implements FxController { private final ObservableValue mountpointDirSupported; private final ObservableValue mountpointDriveLetterSupported; - private final ObservableValue mountpointParentSupported; //TODO: use it in GUI private final ObservableValue readOnlySupported; private final ObservableValue mountFlagsSupported; + private final ObservableValue driveLetter; + private final ObservableValue directoryPath; + + //-- FXML objects -- public CheckBox readOnlyCheckbox; public CheckBox customMountFlagsCheckbox; public TextField mountFlagsField; @@ -49,7 +50,8 @@ public class MountOptionsController implements FxController { public RadioButton mountPointAutoBtn; public RadioButton mountPointDriveLetterBtn; public RadioButton mountPointDirBtn; - public ChoiceBox driveLetterSelection; + public TextField directoryPathField; + public ChoiceBox driveLetterSelection; @Inject MountOptionsController(@VaultOptionsWindow Stage window, @VaultOptionsWindow Vault vault, ObservableValue mountService, WindowsDriveLetters windowsDriveLetters, ResourceBundle resourceBundle, Environment environment) { @@ -57,11 +59,12 @@ public class MountOptionsController implements FxController { this.vault = vault; this.windowsDriveLetters = windowsDriveLetters; this.resourceBundle = resourceBundle; - this.mountpointDirSupported = mountService.map(s -> s.hasCapability(MountCapability.MOUNT_TO_EXISTING_DIR)); + this.mountpointDirSupported = mountService.map(s -> s.hasCapability(MountCapability.MOUNT_TO_EXISTING_DIR) || s.hasCapability(MountCapability.MOUNT_WITHIN_EXISTING_PARENT)); this.mountpointDriveLetterSupported = mountService.map(s -> s.hasCapability(MountCapability.MOUNT_AS_DRIVE_LETTER)); - this.mountpointParentSupported = mountService.map(s -> s.hasCapability(MountCapability.MOUNT_WITHIN_EXISTING_PARENT)); this.mountFlagsSupported = mountService.map(s -> s.hasCapability(MountCapability.MOUNT_FLAGS)); this.readOnlySupported = mountService.map(s -> s.hasCapability(MountCapability.READ_ONLY)); + this.driveLetter = vault.getVaultSettings().mountPoint().map(p -> isDriveLetter(p) ? p : null); + this.directoryPath = vault.getVaultSettings().mountPoint().map(p -> isDriveLetter(p) ? null : p.toString()); } @FXML @@ -73,26 +76,24 @@ public class MountOptionsController implements FxController { mountFlagsField.disableProperty().bind(customMountFlagsCheckbox.selectedProperty().not()); customMountFlagsCheckbox.setSelected(vault.isHavingCustomMountFlags()); - // mount point options: - mountPointToggleGroup.selectedToggleProperty().addListener(this::toggleMountPoint); - driveLetterSelection.getItems().addAll(windowsDriveLetters.getAllDriveLetters()); + //driveLetter choice box + driveLetterSelection.getItems().addAll(windowsDriveLetters.getAll()); driveLetterSelection.setConverter(new WinDriveLetterLabelConverter(windowsDriveLetters, resourceBundle)); - driveLetterSelection.setValue(vault.getVaultSettings().winDriveLetter().get()); + driveLetterSelection.setOnShowing(event -> driveLetterSelection.setConverter(new WinDriveLetterLabelConverter(windowsDriveLetters, resourceBundle))); //TODO: does this work? - if (vault.getVaultSettings().useCustomMountPath().get() && vault.getVaultSettings().getCustomMountPath().isPresent()) { - mountPointToggleGroup.selectToggle(mountPointDirBtn); - } else if (!Strings.isNullOrEmpty(vault.getVaultSettings().winDriveLetter().get())) { - mountPointToggleGroup.selectToggle(mountPointDriveLetterBtn); - } else { + //mountPoint toggle group + var mountPoint = vault.getVaultSettings().getMountPoint(); + if (mountPoint == null) { + //prepare and select auto mountPointToggleGroup.selectToggle(mountPointAutoBtn); + } else if (mountPoint.getParent() == null && isDriveLetter(mountPoint)) { + //prepare and select drive letter + mountPointToggleGroup.selectToggle(mountPointDriveLetterBtn); + } else if (driveLetterSelection.getValue() == null) { + //prepare and select dir + mountPointToggleGroup.selectToggle(mountPointDirBtn); } - - vault.getVaultSettings().useCustomMountPath().bind(mountPointToggleGroup.selectedToggleProperty().isEqualTo(mountPointDirBtn)); - vault.getVaultSettings().winDriveLetter().bind( // - Bindings.when(mountPointToggleGroup.selectedToggleProperty().isEqualTo(mountPointDriveLetterBtn)) // - .then(driveLetterSelection.getSelectionModel().selectedItemProperty()) // - .otherwise((String) null) // - ); + mountPointToggleGroup.selectedToggleProperty().addListener(this::selectedToggleChanged); } @FXML @@ -111,16 +112,29 @@ public class MountOptionsController implements FxController { @FXML public void chooseCustomMountPoint() { - chooseCustomMountPointOrReset(mountPointDirBtn); + try { + Path chosenPath = chooseCustomMountPointInternal(); + vault.getVaultSettings().mountPoint().set(chosenPath); + } catch (NoDirSelectedException e) { + //no-op + } } - private void chooseCustomMountPointOrReset(Toggle previousMountToggle) { + /** + * Prepares and opens a directory chooser dialog. + * This method blocks until the dialog is closed. + * + * @return the absolute path to the chosen directory + * @throws NoDirSelectedException if dialog is closed without choosing a directory + */ + private Path chooseCustomMountPointInternal() throws NoDirSelectedException { DirectoryChooser directoryChooser = new DirectoryChooser(); directoryChooser.setTitle(resourceBundle.getString("vaultOptions.mount.mountPoint.directoryPickerTitle")); try { - var initialDir = Path.of(vault.getVaultSettings().getCustomMountPath().orElse(System.getProperty("user.home"))); + var mp = vault.getVaultSettings().mountPoint().get(); + var initialDir = mp != null && !isDriveLetter(mp) ? mp : Path.of(System.getProperty("user.home")); - if (Files.exists(initialDir)) { + if (Files.isDirectory(initialDir)) { directoryChooser.setInitialDirectory(initialDir.toFile()); } } catch (InvalidPathException e) { @@ -128,49 +142,74 @@ public class MountOptionsController implements FxController { } File file = directoryChooser.showDialog(window); if (file != null) { - vault.getVaultSettings().customMountPath().set(file.getAbsolutePath()); + return file.toPath(); } else { - mountPointToggleGroup.selectToggle(previousMountToggle); + throw new NoDirSelectedException(); } } - private void toggleMountPoint(@SuppressWarnings("unused") ObservableValue observable, Toggle oldValue, Toggle newValue) { - if (mountPointDirBtn.equals(newValue) && Strings.isNullOrEmpty(vault.getVaultSettings().customMountPath().get())) { - chooseCustomMountPointOrReset(oldValue); + private void selectedToggleChanged(ObservableValue observable, Toggle oldToggle, Toggle newToggle) { + Path mountPointToBe = null; + try { + //Remark: the mountpoint corresponding to the newToggle must be null, otherwise it would not be new! + if (mountPointDriveLetterBtn.equals(newToggle)) { + mountPointToBe = driveLetterSelection.getItems().get(0); + } else if (mountPointDirBtn.equals(newToggle)) { + mountPointToBe = chooseCustomMountPointInternal(); + } + vault.getVaultSettings().mountPoint().set(mountPointToBe); + } catch (NoDirSelectedException e) { + if (!mountPointDirBtn.equals(oldToggle)) { + mountPointToggleGroup.selectToggle(oldToggle); + + } } } - /** - * Converts 'C' to "C:" to translate between model and GUI. - */ - private static class WinDriveLetterLabelConverter extends StringConverter { + private boolean isDriveLetter(Path mountPoint) { + if (mountPoint != null) { + var s = mountPoint.toString(); + return s.length() == 3 && mountPoint.toString().endsWith(":\\"); + } + return false; + } - private final Set occupiedDriveLetters; + private static class WinDriveLetterLabelConverter extends StringConverter { + + private final Set occupiedDriveLetters; private final ResourceBundle resourceBundle; WinDriveLetterLabelConverter(WindowsDriveLetters windowsDriveLetters, ResourceBundle resourceBundle) { - this.occupiedDriveLetters = windowsDriveLetters.getOccupiedDriveLetters(); + this.occupiedDriveLetters = windowsDriveLetters.getOccupied(); this.resourceBundle = resourceBundle; } @Override - public String toString(String driveLetter) { - if (Strings.isNullOrEmpty(driveLetter)) { + public String toString(Path driveLetter) { + if (driveLetter == null) { return ""; } else if (occupiedDriveLetters.contains(driveLetter)) { - return driveLetter + ": (" + resourceBundle.getString("vaultOptions.mount.winDriveLetterOccupied") + ")"; + return driveLetter.toString().substring(0, 2) + " (" + resourceBundle.getString("vaultOptions.mount.winDriveLetterOccupied") + ")"; } else { - return driveLetter + ":"; + return driveLetter.toString().substring(0, 2); } } @Override - public String fromString(String string) { - throw new UnsupportedOperationException(); + public Path fromString(String string) { + if (string.isEmpty()) { + return null; + } else { + return Path.of(string + "\\"); + } } } + //@formatter:off + private static class NoDirSelectedException extends Exception {} + //@formatter:on + // Getter & Setter public ObservableValue mountFlagsSupportedProperty() { @@ -189,14 +228,6 @@ public class MountOptionsController implements FxController { return mountpointDirSupported.getValue(); } - public ObservableValue mountpointParentSupportedProperty() { - return mountpointParentSupported; - } - - public boolean isMountpointParentSupported() { - return mountpointParentSupported.getValue(); - } - public ObservableValue mountpointDriveLetterSupportedProperty() { return mountpointDriveLetterSupported; } @@ -213,9 +244,20 @@ public class MountOptionsController implements FxController { return readOnlySupported.getValue(); } + public ObservableValue driveLetterProperty() { + return driveLetter; + } - public String getCustomMountPath() { - return vault.getVaultSettings().customMountPath().get(); + public Path getDriveLetter() { + return driveLetter.getValue(); + } + + public ObservableValue directoryPathProperty() { + return directoryPath; + } + + public String getDirectoryPath() { + return directoryPath.getValue(); } } diff --git a/src/main/resources/fxml/vault_options_mount.fxml b/src/main/resources/fxml/vault_options_mount.fxml index 92221afcb..5119724c2 100644 --- a/src/main/resources/fxml/vault_options_mount.fxml +++ b/src/main/resources/fxml/vault_options_mount.fxml @@ -44,18 +44,18 @@ - + - + - - + diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index b3ac7473a..8a1e6f8de 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -382,9 +382,9 @@ vaultOptions.mount.winDriveLetterOccupied=occupied vaultOptions.mount.mountPoint=Mount Point vaultOptions.mount.mountPoint.auto=Automatically pick a suitable location vaultOptions.mount.mountPoint.driveLetter=Use assigned drive letter -vaultOptions.mount.mountPoint.custom=Custom path +vaultOptions.mount.mountPoint.custom=Use chosen directory vaultOptions.mount.mountPoint.directoryPickerButton=Choose… -vaultOptions.mount.mountPoint.directoryPickerTitle=Pick an empty directory +vaultOptions.mount.mountPoint.directoryPickerTitle=Pick a directory ## Master Key vaultOptions.masterkey=Password vaultOptions.masterkey.changePasswordBtn=Change Password diff --git a/src/test/java/org/cryptomator/common/settings/VaultSettingsJsonAdapterTest.java b/src/test/java/org/cryptomator/common/settings/VaultSettingsJsonAdapterTest.java index 3d902ba6c..b7a0dade4 100644 --- a/src/test/java/org/cryptomator/common/settings/VaultSettingsJsonAdapterTest.java +++ b/src/test/java/org/cryptomator/common/settings/VaultSettingsJsonAdapterTest.java @@ -34,8 +34,6 @@ public class VaultSettingsJsonAdapterTest { () -> assertEquals("foo", vaultSettings.getId()), () -> assertEquals(Paths.get("/foo/bar"), vaultSettings.path().get()), () -> assertEquals("test", vaultSettings.displayName().get()), - () -> assertEquals("X", vaultSettings.winDriveLetter().get()), - () -> assertEquals("/home/test/crypto", vaultSettings.customMountPath().get()), () -> assertEquals("--foo --bar", vaultSettings.mountFlags().get()) ); }