enhance message

old code can still run it on fx thread

src/main/java/org/cryptomator/common/vaults/NotAVaultDirectoryException.java

@@ -0,0 +1,32 @@
+package org.cryptomator.common.vaults;
+
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+
+public class NotAVaultDirectoryException extends NoSuchFileException {
+
+	public enum Reason {
+		MISSING_DATA_DIR,
+		DATA_NOT_A_DIRECTORY,
+		MISSING_VAULT_CONFIG,
+		VAULT_CONFIG_ACCESS_DENIED,
+		UNSUPPORTED_STRUCTURE
+	}
+
+	private final transient Path path;
+	private final Reason reason;
+
+	public NotAVaultDirectoryException(Path path, Reason reason) {
+		super(path.toString(), null, "Not a vault directory: " + reason);
+		this.path = path;
+		this.reason = reason;
+	}
+
+	public Path path() {
+		return path;
+	}
+
+	public Reason notAVaultReason() {
+		return reason;
+	}
+}

src/main/java/org/cryptomator/common/vaults/VaultListManager.java

@@ -22,20 +22,20 @@ import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javafx.application.Platform;
 import javafx.collections.ObservableList;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.ResourceBundle;
 
 import static org.cryptomator.common.Constants.MASTERKEY_FILENAME;
 import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME;
 import static org.cryptomator.common.vaults.VaultState.Value.*;
+import static org.cryptomator.cryptofs.common.Constants.DATA_DIR_NAME;
 
 @Singleton
 public class VaultListManager {
@@ -72,20 +72,51 @@ public class VaultListManager {
 		return vaultList.stream().anyMatch(v -> vaultPath.equals(v.getPath()));
 	}
 
+	/**
+	 * Safe to call from any thread: the IO work runs on the calling thread, but the
+	 * {@code ObservableList} mutation is marshaled to the JavaFX application thread.
+	 */
 	public Vault add(Path pathToVault) throws IOException {
 		Path normalizedPathToVault = pathToVault.normalize().toAbsolutePath();
-		if (CryptoFileSystemProvider.checkDirStructureForVault(normalizedPathToVault, VAULTCONFIG_FILENAME, MASTERKEY_FILENAME) == DirStructure.UNRELATED) {
-			throw new NoSuchFileException(normalizedPathToVault.toString(), null, "Not a vault directory");
-		}
+		assertIsVaultDirectory(normalizedPathToVault);
 
 		return get(normalizedPathToVault) //
 				.orElseGet(() -> {
 					Vault newVault = create(newVaultSettings(normalizedPathToVault));
-					vaultList.add(newVault);
+					if (Platform.isFxApplicationThread()) {
+						vaultList.add(newVault);
+					} else {
+						Platform.runLater(() -> vaultList.add(newVault));
+					}
 					return newVault;
 				});
 	}
 
+	public static void assertIsVaultDirectory(Path pathToVault) throws IOException {
+		if (CryptoFileSystemProvider.checkDirStructureForVault(pathToVault, VAULTCONFIG_FILENAME, MASTERKEY_FILENAME) == DirStructure.UNRELATED) {
+			Path dataDir = pathToVault.resolve(DATA_DIR_NAME);
+			if (!Files.isDirectory(dataDir)) {
+				if (Files.exists(dataDir)) {
+					throw new NotAVaultDirectoryException(pathToVault, NotAVaultDirectoryException.Reason.DATA_NOT_A_DIRECTORY);
+				} else {
+					throw new NotAVaultDirectoryException(pathToVault, NotAVaultDirectoryException.Reason.MISSING_DATA_DIR);
+				}
+			}
+
+			Path vaultConfig = pathToVault.resolve(VAULTCONFIG_FILENAME);
+			if (!Files.isReadable(vaultConfig)) {
+				if (Files.exists(vaultConfig)) {
+					throw new NotAVaultDirectoryException(pathToVault, NotAVaultDirectoryException.Reason.VAULT_CONFIG_ACCESS_DENIED);
+				} else {
+					throw new NotAVaultDirectoryException(pathToVault, NotAVaultDirectoryException.Reason.MISSING_VAULT_CONFIG);
+				}
+			}
+
+			//if vault is legacy _and_ not readable, just say unsupported
+			throw new NotAVaultDirectoryException(pathToVault, NotAVaultDirectoryException.Reason.UNSUPPORTED_STRUCTURE);
+		}
+	}
+
 	private VaultSettings newVaultSettings(Path path) {
 		VaultSettings vaultSettings = VaultSettings.withRandomId();
 		vaultSettings.path.set(path);
@@ -153,7 +184,7 @@ public class VaultListManager {
 				//for legacy reasons: pre v8 vault do not have a config, but they are in the NEEDS_MIGRATION state
 				vaultSettings.lastKnownKeyLoader.set(MasterkeyFileLoadingStrategy.SCHEME);
 			}
-			case VAULT_CONFIG_MISSING ->  {
+			case VAULT_CONFIG_MISSING -> {
 				//Nothing to do here, since there is no config to read
 			}
 			case MISSING, ALL_MISSING, ERROR, PROCESSING -> {

src/main/java/org/cryptomator/ui/addvaultwizard/ChooseExistingVaultController.java

@@ -2,12 +2,14 @@ package org.cryptomator.ui.addvaultwizard;
 
 import dagger.Lazy;
 import org.apache.commons.lang3.SystemUtils;
+import org.cryptomator.common.vaults.NotAVaultDirectoryException;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultListManager;
 import org.cryptomator.integrations.uiappearance.Theme;
 import org.cryptomator.ui.common.FxController;
 import org.cryptomator.ui.common.FxmlFile;
 import org.cryptomator.ui.common.FxmlScene;
+import org.cryptomator.ui.dialogs.Dialogs;
 import org.cryptomator.ui.fxapp.FxApplicationStyle;
 import org.cryptomator.ui.fxapp.FxApplicationWindows;
 import org.slf4j.Logger;
@@ -41,6 +43,7 @@ public class ChooseExistingVaultController implements FxController {
 	private final ObjectProperty<Vault> vault;
 	private final VaultListManager vaultListManager;
 	private final ResourceBundle resourceBundle;
+	private final Dialogs dialogs;
 	private final ObservableValue<Image> screenshot;
 
 	@Inject
@@ -51,6 +54,7 @@ public class ChooseExistingVaultController implements FxController {
 								  @AddVaultWizardWindow ObjectProperty<Vault> vault, //
 								  VaultListManager vaultListManager, //
 								  ResourceBundle resourceBundle, //
+								  Dialogs dialogs, //
 								  FxApplicationStyle applicationStyle) {
 		this.window = window;
 		this.successScene = successScene;
@@ -59,6 +63,7 @@ public class ChooseExistingVaultController implements FxController {
 		this.vault = vault;
 		this.vaultListManager = vaultListManager;
 		this.resourceBundle = resourceBundle;
+		this.dialogs = dialogs;
 		this.screenshot = applicationStyle.appliedAppThemeProperty().map(this::selectScreenshot);
 	}
 
@@ -87,6 +92,9 @@ public class ChooseExistingVaultController implements FxController {
 				Vault newVault = vaultListManager.add(vaultPath.get());
 				vault.set(newVault);
 				window.setScene(successScene.get());
+			} catch (NotAVaultDirectoryException e) {
+				LOG.warn("Selected folder is not a vault directory: {}", e.getMessage());
+				dialogs.prepareNotAVaultDirectoryDialog(window, e).build().showAndWait();
 			} catch (IOException e) {
 				LOG.error("Failed to open existing vault.", e);
 				appWindows.showErrorWindow(e, window, window.getScene());

src/main/java/org/cryptomator/ui/dialogs/Dialogs.java

@@ -1,6 +1,7 @@
 package org.cryptomator.ui.dialogs;
 
 import org.cryptomator.common.settings.Settings;
+import org.cryptomator.common.vaults.NotAVaultDirectoryException;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.ui.common.DefaultSceneFactory;
 import org.cryptomator.ui.common.StageFactory;
@@ -139,6 +140,24 @@ public class Dialogs {
 				.setCancelAction(Stage::close);
 	}
 
+	public SimpleDialog.Builder prepareNotAVaultDirectoryDialog(Stage window, NotAVaultDirectoryException e) {
+		String descriptionKey = switch (e.notAVaultReason()) {
+			case MISSING_DATA_DIR -> "addvaultwizard.existing.notAVault.description.missingDataDir";
+			case DATA_NOT_A_DIRECTORY -> "addvaultwizard.existing.notAVault.description.dataNotADirectory";
+			case MISSING_VAULT_CONFIG -> "addvaultwizard.existing.notAVault.description.missingVaultConfig";
+			case VAULT_CONFIG_ACCESS_DENIED -> "addvaultwizard.existing.notAVault.description.vaultConfigAccessDenied";
+			case UNSUPPORTED_STRUCTURE -> "addvaultwizard.existing.notAVault.description.unsupportedStructure";
+		};
+		return createDialogBuilder() //
+				.setOwner(window) //
+				.setTitleKey("addvaultwizard.existing.notAVault.title") //
+				.setMessageKey("addvaultwizard.existing.notAVault.message") //
+				.setDescriptionKey(descriptionKey, e.path().getFileName() != null ? e.path().getFileName().toString() : e.path().toString()) //
+				.setIcon(FontAwesome5Icon.EXCLAMATION) //
+				.setOkButtonKey(BUTTON_KEY_CLOSE) //
+				.setOkAction(Stage::close);
+	}
+
 	public SimpleDialog.Builder prepareNoDDirectorySelectedDialog(Stage window) {
 		return createDialogBuilder() //
 				.setOwner(window) //

src/main/java/org/cryptomator/ui/fxapp/AppLaunchEventHandler.java

@@ -1,15 +1,18 @@
 package org.cryptomator.ui.fxapp;
 
+import org.cryptomator.common.vaults.NotAVaultDirectoryException;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultListManager;
 import org.cryptomator.launcher.AppLaunchEvent;
 import org.cryptomator.ui.common.VaultService;
+import org.cryptomator.ui.dialogs.Dialogs;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import javax.inject.Named;
 import javafx.application.Platform;
+import javafx.stage.Stage;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Optional;
@@ -29,14 +32,18 @@ class AppLaunchEventHandler {
 	private final FxApplicationWindows appWindows;
 	private final VaultListManager vaultListManager;
 	private final VaultService vaultService;
+	private final Stage primaryStage;
+	private final Dialogs dialogs;
 
 	@Inject
-	public AppLaunchEventHandler(@Named("launchEventQueue") BlockingQueue<AppLaunchEvent> launchEventQueue, ExecutorService executorService, FxApplicationWindows appWindows, VaultListManager vaultListManager, VaultService vaultService) {
+	public AppLaunchEventHandler(@Named("launchEventQueue") BlockingQueue<AppLaunchEvent> launchEventQueue, ExecutorService executorService, FxApplicationWindows appWindows, VaultListManager vaultListManager, VaultService vaultService, @PrimaryStage Stage primaryStage, Dialogs dialogs) {
 		this.launchEventQueue = launchEventQueue;
 		this.executorService = executorService;
 		this.appWindows = appWindows;
 		this.vaultListManager = vaultListManager;
 		this.vaultService = vaultService;
+		this.primaryStage = primaryStage;
+		this.dialogs = dialogs;
 	}
 
 	public void startHandlingLaunchEvents() {
@@ -58,31 +65,34 @@ class AppLaunchEventHandler {
 	private void handleLaunchEvent(AppLaunchEvent event) {
 		switch (event.type()) {
 			case REVEAL_APP -> appWindows.showMainWindow();
-			case OPEN_FILE -> Platform.runLater(() -> {
-				event.pathsToOpen().forEach(this::openPotentialVault);
-			});
+			case OPEN_FILE -> event.pathsToOpen().forEach(this::openPotentialVault);
 			default -> LOG.warn("Unsupported event type: {}", event.type());
 		}
 	}
 
 	// TODO deduplicate MainWindowController...
 	private void openPotentialVault(Path path) {
-		assert Platform.isFxApplicationThread();
-		try {
-			Path potentialVaultPath = path.getFileName().toString().endsWith(CRYPTOMATOR_FILENAME_EXT) ? path.getParent() : path;
-			final Optional<Vault> v = vaultListManager.get(potentialVaultPath);
-			if (v.isPresent()) {
-				if (v.get().isUnlocked()) {
-					vaultService.reveal(v.get());
-				} else if (v.get().isLocked()) {
-					appWindows.startUnlockWorkflow(v.get(), null);
+		assert !Platform.isFxApplicationThread();
+		Path potentialVaultPath = path.getFileName().toString().endsWith(CRYPTOMATOR_FILENAME_EXT) ? path.getParent() : path;
+		Optional<Vault> existing = vaultListManager.get(potentialVaultPath.normalize().toAbsolutePath());
+		if (existing.isPresent()) {
+			Platform.runLater(() -> {
+				if (existing.get().isUnlocked()) {
+					vaultService.reveal(existing.get());
+				} else if (existing.get().isLocked()) {
+					appWindows.startUnlockWorkflow(existing.get(), null);
 				}
-			} else {
-				vaultListManager.add(potentialVaultPath);
-				LOG.debug("Added vault {}", potentialVaultPath);
-			}
+			});
+			return;
+		}
+		try {
+			vaultListManager.add(potentialVaultPath);
+			LOG.debug("Added vault {}", potentialVaultPath);
+		} catch (NotAVaultDirectoryException e) {
+			LOG.warn("Cannot add {}: {}", potentialVaultPath, e.getMessage());
+			Platform.runLater(() -> dialogs.prepareNotAVaultDirectoryDialog(primaryStage, e).build().showAndWait());
 		} catch (IOException e) {
-			LOG.error("Failed to add vault " + path, e);
+			LOG.error("Failed to add vault {}", potentialVaultPath, e);
 		}
 	}
 

src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java

@@ -4,6 +4,7 @@ import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.common.recovery.RecoveryActionType;
 import org.cryptomator.common.recovery.VaultPreparator;
 import org.cryptomator.common.settings.Settings;
+import org.cryptomator.common.vaults.NotAVaultDirectoryException;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.common.vaults.VaultComponent;
 import org.cryptomator.common.vaults.VaultListManager;
@@ -23,6 +24,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
+import javafx.application.Platform;
 import javafx.beans.binding.Bindings;
 import javafx.beans.binding.BooleanBinding;
 import javafx.beans.property.BooleanProperty;
@@ -55,6 +57,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.ResourceBundle;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
 import java.util.stream.Collectors;
 
 import static org.cryptomator.common.Constants.CRYPTOMATOR_FILENAME_EXT;
@@ -90,6 +93,7 @@ public class VaultListController implements FxController {
 	private final VaultComponent.Factory vaultComponentFactory;
 	private final RecoveryKeyComponent.Factory recoveryKeyWindow;
 	private final List<MountService> mountServices;
+	private final ExecutorService executor;
 
 	public ListView<Vault> vaultList;
 	public StackPane root;
@@ -113,7 +117,8 @@ public class VaultListController implements FxController {
 						RecoveryKeyComponent.Factory recoveryKeyWindow, //
 						VaultComponent.Factory vaultComponentFactory, //
 						List<MountService> mountServices, //
-						FxFSEventList fxFSEventList) {
+						FxFSEventList fxFSEventList, //
+						ExecutorService executor) {
 		this.mainWindow = mainWindow;
 		this.vaults = vaults;
 		this.selectedVault = selectedVault;
@@ -127,6 +132,7 @@ public class VaultListController implements FxController {
 		this.recoveryKeyWindow = recoveryKeyWindow;
 		this.vaultComponentFactory = vaultComponentFactory;
 		this.mountServices = mountServices;
+		this.executor = executor;
 
 		this.emptyVaultList = Bindings.isEmpty(vaults);
 		this.unreadEvents = fxFSEventList.unreadEventsProperty();
@@ -324,15 +330,17 @@ public class VaultListController implements FxController {
 	}
 
 	private void addVault(Path pathToVault) {
-		try {
-			if (pathToVault.getFileName().toString().endsWith(CRYPTOMATOR_FILENAME_EXT)) {
-				vaultListManager.add(pathToVault.getParent());
-			} else {
-				vaultListManager.add(pathToVault);
+		Path target = pathToVault.getFileName().toString().endsWith(CRYPTOMATOR_FILENAME_EXT) ? pathToVault.getParent() : pathToVault;
+		executor.execute(() -> {
+			try {
+				vaultListManager.add(target);
+			} catch (NotAVaultDirectoryException e) {
+				LOG.warn("Cannot add {}: {}", target, e.getMessage());
+				Platform.runLater(() -> dialogs.prepareNotAVaultDirectoryDialog(mainWindow, e).build().showAndWait());
+			} catch (IOException e) {
+				LOG.error("Failed to add vault {}", target, e);
 			}
-		} catch (IOException e) {
-			LOG.debug("Not a vault: {}", pathToVault);
-		}
+		});
 	}
 
 	@FXML

src/main/resources/i18n/strings.properties

@@ -110,6 +110,13 @@ addvaultwizard.existing.restore=Restore…
 addvaultwizard.existing.chooseBtn=Choose…
 addvaultwizard.existing.filePickerTitle=Select Vault File
 addvaultwizard.existing.filePickerMimeDesc=Cryptomator Vault
+addvaultwizard.existing.notAVault.title=Not a Vault
+addvaultwizard.existing.notAVault.message=The selected folder is not a Cryptomator vault
+addvaultwizard.existing.notAVault.description.missingDataDir=The required "d" subdirectory is missing inside "%s".
+addvaultwizard.existing.notAVault.description.dataNotADirectory=The "d" entry inside "%s" is not a directory.
+addvaultwizard.existing.notAVault.description.missingVaultConfig=The required "vault.cryptomator" file is missing inside "%s".
+addvaultwizard.existing.notAVault.description.vaultConfigAccessDenied=File "vault.cryptomator" inside "%s" cannot be read due to insufficient access rights.
+addvaultwizard.existing.notAVault.description.unsupportedStructure=The directory structure of "%s" is not supported.
 ## Success
 addvaultwizard.success.nextStepsInstructions=Added vault "%s".\nYou need to unlock this vault to access or add contents. Alternatively you can unlock it at any later point in time.
 addvaultwizard.success.unlockNow=Unlock Now

src/test/java/org/cryptomator/common/vaults/VaultListManagerTest.java

@@ -0,0 +1,70 @@
+package org.cryptomator.common.vaults;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.cryptomator.common.Constants.MASTERKEY_FILENAME;
+import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME;
+import static org.cryptomator.cryptofs.common.Constants.DATA_DIR_NAME;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class VaultListManagerTest {
+
+	@Test
+	void testAssertIsVaultDirectoryWhenDataDirIsMissing(@TempDir Path tmpDir) {
+		NotAVaultDirectoryException e = assertThrows(NotAVaultDirectoryException.class, () -> {
+			VaultListManager.assertIsVaultDirectory(tmpDir);
+		});
+
+		assertEquals(NotAVaultDirectoryException.Reason.MISSING_DATA_DIR, e.notAVaultReason());
+	}
+
+	@Test
+	void testAssertIsVaultDirectoryWhenDataDirIsFile(@TempDir Path tmpDir) throws IOException {
+		Files.createFile(tmpDir.resolve(DATA_DIR_NAME));
+
+		NotAVaultDirectoryException e = assertThrows(NotAVaultDirectoryException.class, () -> {
+			VaultListManager.assertIsVaultDirectory(tmpDir);
+		});
+
+		assertEquals(NotAVaultDirectoryException.Reason.DATA_NOT_A_DIRECTORY, e.notAVaultReason());
+	}
+
+	@Test
+	void testAssertIsVaultDirectoryWhenVaultConfigAndMasterkeyAreMissing(@TempDir Path tmpDir) throws IOException {
+		Files.createDirectory(tmpDir.resolve(DATA_DIR_NAME));
+
+		NotAVaultDirectoryException e = assertThrows(NotAVaultDirectoryException.class, () -> {
+			VaultListManager.assertIsVaultDirectory(tmpDir);
+		});
+
+		assertEquals(NotAVaultDirectoryException.Reason.MISSING_VAULT_CONFIG, e.notAVaultReason());
+	}
+
+	@Test
+	void testAssertIsVaultDirectoryAcceptsModernVault(@TempDir Path tmpDir) throws IOException {
+		Files.createDirectory(tmpDir.resolve(DATA_DIR_NAME));
+		Files.createFile(tmpDir.resolve(VAULTCONFIG_FILENAME));
+
+		assertDoesNotThrow(() -> {
+			VaultListManager.assertIsVaultDirectory(tmpDir);
+		});
+	}
+
+	@Test
+	void testAssertIsVaultDirectoryAcceptsLegacyVaultCandidate(@TempDir Path tmpDir) throws IOException {
+		Files.createDirectory(tmpDir.resolve(DATA_DIR_NAME));
+		Files.createFile(tmpDir.resolve(MASTERKEY_FILENAME));
+
+		assertDoesNotThrow(() -> {
+			VaultListManager.assertIsVaultDirectory(tmpDir);
+		});
+	}
+
+}