Skip stale unlock and lock requests instead of showing the generic error window

This commit is contained in:
Tobias Hagemann
2026-10-15 09:00:11 +02:00
parent b88fd30813
commit 5d7b5f75d8
5 changed files with 207 additions and 28 deletions
+1
View File
@@ -18,6 +18,7 @@ Changes to prior versions can be found on the [Github release page](https://gith
* Fixed background threads piling up with every unlock and lock of a vault
* Fixed vaults showing as unlocked while still locking when locking several vaults at once, for example on quit, and one of them failed to lock
* Fixed a vault showing as locked while unlocked when the tray menu was opened during its unlock
* Fixed an error window ("Vault not locked." or "Vault not unlocked.") appearing when a vault was unlocked or locked twice in quick succession, for example from a stale tray menu
### Changed
* Improved usability of the trust on first use dialog ([#4295](https://github.com/cryptomator/cryptomator/pull/4295))
@@ -10,6 +10,7 @@ import org.cryptomator.ui.dialogs.SimpleDialog;
import org.cryptomator.ui.error.ErrorComponent;
import org.cryptomator.ui.eventview.EventViewComponent;
import org.cryptomator.ui.lock.LockComponent;
import org.cryptomator.ui.lock.LockWorkflow;
import org.cryptomator.ui.mainwindow.MainWindowComponent;
import org.cryptomator.ui.notification.NotificationComponent;
import org.cryptomator.ui.preferences.PreferencesComponent;
@@ -170,11 +171,14 @@ public class FxApplicationWindows {
public CompletionStage<Void> startUnlockWorkflow(Vault vault, @Nullable Stage owner) {
return CompletableFuture.supplyAsync(() -> {
Preconditions.checkState(vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING), "Vault not locked.");
if (!vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING)) {
LOG.warn("Vault '{}' is {} instead of LOCKED. Skipping unlock.", vault.getDisplayName(), vault.stateProperty().get());
return Optional.<UnlockWorkflow>empty();
}
LOG.debug("Start unlock workflow for {}", vault.getDisplayName());
return unlockWorkflowFactory.create(vault, owner).unlockWorkflow();
return Optional.of(unlockWorkflowFactory.create(vault, owner).unlockWorkflow());
}, Platform::runLater) //
.thenAcceptAsync(UnlockWorkflow::run, executor).exceptionally(e -> {
.thenAcceptAsync(unlockWorkflow -> unlockWorkflow.ifPresent(UnlockWorkflow::run), executor).exceptionally(e -> {
showErrorWindow(e, owner == null ? primaryStage : owner, null);
return null;
});
@@ -182,12 +186,14 @@ public class FxApplicationWindows {
public CompletionStage<Void> startLockWorkflow(Vault vault, @Nullable Stage owner) {
return CompletableFuture.supplyAsync(() -> {
Preconditions.checkState(vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING), "Vault not unlocked.");
if (!vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING)) {
LOG.warn("Vault '{}' is {} instead of UNLOCKED. Skipping lock.", vault.getDisplayName(), vault.stateProperty().get());
return Optional.<LockWorkflow>empty();
}
LOG.debug("Start lock workflow for {}", vault.getDisplayName());
return lockWorkflowFactory.create(vault, owner).lockWorkflow();
return Optional.of(lockWorkflowFactory.create(vault, owner).lockWorkflow());
}, Platform::runLater) //
.thenCompose(lockWorkflow -> CompletableFuture.runAsync(lockWorkflow, executor)) //
.exceptionally(e -> {
.thenAcceptAsync(lockWorkflow -> lockWorkflow.ifPresent(LockWorkflow::run), executor).exceptionally(e -> {
showErrorWindow(e, owner == null ? primaryStage : owner, null);
return null;
});
@@ -170,7 +170,7 @@ public class UnlockWorkflow extends Task<Void> {
var readOnlyDialog = dialogs.prepareRetryIfReadonlyDialog(mainWindow, stage -> {
stage.close();
vault.getVaultSettings().usesReadOnlyMode.set(true);
restartUnlock();
appWindows.startUnlockWorkflow(vault, owner);
}).build();
Platform.runLater(readOnlyDialog::showAndWait);
@@ -179,7 +179,7 @@ public class UnlockWorkflow extends Task<Void> {
private void handleExtensionSwitchedOff() {
var extensionSwitchedOffDialog = dialogs.prepareFSKitExtensionSwitchedOffDialog(mainWindow, vault, stage -> {
stage.close();
restartUnlock();
appWindows.startUnlockWorkflow(vault, owner);
}).build();
Platform.runLater(() -> {
@@ -188,14 +188,6 @@ public class UnlockWorkflow extends Task<Void> {
});
}
private void restartUnlock() {
if (!vault.isLocked()) {
LOG.warn("Vault '{}' is {} instead of LOCKED. Aborting unlock retry.", vault.getDisplayName(), vault.getState());
return;
}
appWindows.startUnlockWorkflow(vault, owner);
}
@Override
protected void cancelled() {
LOG.debug("Unlock of '{}' canceled.", vault.getDisplayName());
@@ -0,0 +1,191 @@
package org.cryptomator.ui.fxapp;
import org.cryptomator.common.vaults.Vault;
import org.cryptomator.common.vaults.VaultState;
import org.cryptomator.ui.dialogs.Dialogs;
import org.cryptomator.ui.error.ErrorComponent;
import org.cryptomator.ui.eventview.EventViewComponent;
import org.cryptomator.ui.lock.LockComponent;
import org.cryptomator.ui.lock.LockWorkflow;
import org.cryptomator.ui.mainwindow.MainWindowComponent;
import org.cryptomator.ui.notification.NotificationComponent;
import org.cryptomator.ui.preferences.PreferencesComponent;
import org.cryptomator.ui.quit.QuitComponent;
import org.cryptomator.ui.sharevault.ShareVaultComponent;
import org.cryptomator.ui.unlock.UnlockComponent;
import org.cryptomator.ui.unlock.UnlockWorkflow;
import org.cryptomator.ui.updatereminder.UpdateReminderComponent;
import org.cryptomator.ui.vaultoptions.VaultOptionsComponent;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import javafx.application.Platform;
import javafx.stage.Stage;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.ExecutorService;
class FxApplicationWindowsTest {
private final Stage primaryStage = Mockito.mock(Stage.class);
private final Stage owner = Mockito.mock(Stage.class);
private final Vault vault = Mockito.mock(Vault.class);
private final UnlockComponent.Factory unlockWorkflowFactory = Mockito.mock(UnlockComponent.Factory.class);
private final UnlockWorkflow unlockWorkflow = Mockito.mock(UnlockWorkflow.class);
private final LockComponent.Factory lockWorkflowFactory = Mockito.mock(LockComponent.Factory.class);
private final LockWorkflow lockWorkflow = Mockito.mock(LockWorkflow.class);
private final ErrorComponent.Factory errorWindowFactory = Mockito.mock(ErrorComponent.Factory.class);
private final ExecutorService executor = Mockito.mock(ExecutorService.class);
private final List<Runnable> queuedTasks = new ArrayList<>();
private MockedStatic<Platform> platform;
private FxApplicationWindows appWindows;
@BeforeEach
public void setup() {
platform = Mockito.mockStatic(Platform.class);
platform.when(() -> Platform.runLater(Mockito.any())).thenAnswer(invocation -> {
invocation.getArgument(0, Runnable.class).run();
return null;
});
Mockito.doAnswer(invocation -> queuedTasks.add(invocation.getArgument(0))).when(executor).execute(Mockito.any());
Mockito.when(vault.getDisplayName()).thenReturn("vault");
var unlockComponent = Mockito.mock(UnlockComponent.class);
Mockito.when(unlockComponent.unlockWorkflow()).thenReturn(unlockWorkflow);
Mockito.when(unlockWorkflowFactory.create(Mockito.eq(vault), Mockito.any())).thenReturn(unlockComponent);
var lockComponent = Mockito.mock(LockComponent.class);
Mockito.when(lockComponent.lockWorkflow()).thenReturn(lockWorkflow);
Mockito.when(lockWorkflowFactory.create(Mockito.eq(vault), Mockito.any())).thenReturn(lockComponent);
Mockito.when(errorWindowFactory.create(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(Mockito.mock(ErrorComponent.class));
appWindows = new FxApplicationWindows(primaryStage, Optional.empty(), Mockito.mock(MainWindowComponent.Builder.class), Mockito.mock(PreferencesComponent.Builder.class), Mockito.mock(QuitComponent.Builder.class), unlockWorkflowFactory, Mockito.mock(UpdateReminderComponent.Factory.class), lockWorkflowFactory, errorWindowFactory, Mockito.mock(VaultOptionsComponent.Factory.class), Mockito.mock(ShareVaultComponent.Factory.class), Mockito.mock(EventViewComponent.Factory.class), Mockito.mock(NotificationComponent.Factory.class), executor, Mockito.mock(Dialogs.class));
}
@AfterEach
public void tearDown() {
platform.close();
}
private VaultState givenVaultState(VaultState.Value value) {
var state = new VaultState(value);
Mockito.when(vault.stateProperty()).thenReturn(state);
return state;
}
private void runQueuedTasks() {
var tasks = List.copyOf(queuedTasks);
queuedTasks.clear();
tasks.forEach(Runnable::run);
}
private static void assertCompletedNormally(CompletionStage<Void> stage) {
var future = stage.toCompletableFuture();
Assertions.assertTrue(future.isDone());
Assertions.assertFalse(future.isCompletedExceptionally());
}
@Test
public void testSecondUnlockStartWhileProcessingSkips() {
var state = givenVaultState(VaultState.Value.LOCKED);
var firstUnlock = appWindows.startUnlockWorkflow(vault, null);
var secondUnlock = appWindows.startUnlockWorkflow(vault, null);
runQueuedTasks();
Mockito.verify(unlockWorkflowFactory).create(vault, null);
Mockito.verify(unlockWorkflow).run();
Mockito.verifyNoInteractions(errorWindowFactory);
assertCompletedNormally(firstUnlock);
assertCompletedNormally(secondUnlock);
Assertions.assertEquals(VaultState.Value.PROCESSING, state.get());
}
@Test
public void testStaleLockStartSkips() {
var state = givenVaultState(VaultState.Value.LOCKED);
var lock = appWindows.startLockWorkflow(vault, null);
runQueuedTasks();
Mockito.verifyNoInteractions(lockWorkflowFactory, errorWindowFactory);
assertCompletedNormally(lock);
Assertions.assertEquals(VaultState.Value.LOCKED, state.get());
}
@Test
public void testUnlockCompletesAfterWorkflowRan() {
givenVaultState(VaultState.Value.LOCKED);
var unlock = appWindows.startUnlockWorkflow(vault, owner);
Mockito.verify(unlockWorkflowFactory).create(vault, owner);
Assertions.assertFalse(unlock.toCompletableFuture().isDone());
Mockito.verify(unlockWorkflow, Mockito.never()).run();
runQueuedTasks();
Mockito.verify(unlockWorkflow).run();
assertCompletedNormally(unlock);
}
@Test
public void testLockCompletesAfterWorkflowRan() {
givenVaultState(VaultState.Value.UNLOCKED);
var lock = appWindows.startLockWorkflow(vault, owner);
Mockito.verify(lockWorkflowFactory).create(vault, owner);
Assertions.assertFalse(lock.toCompletableFuture().isDone());
Mockito.verify(lockWorkflow, Mockito.never()).run();
runQueuedTasks();
Mockito.verify(lockWorkflow).run();
assertCompletedNormally(lock);
}
@Test
public void testUnlockFactoryFailureShowsErrorInOwner() {
givenVaultState(VaultState.Value.LOCKED);
Mockito.when(unlockWorkflowFactory.create(vault, owner)).thenThrow(new RuntimeException("factory failed"));
appWindows.startUnlockWorkflow(vault, owner);
runQueuedTasks();
Mockito.verify(errorWindowFactory).create(Mockito.any(), Mockito.eq(owner), Mockito.isNull());
}
@Test
public void testUnlockFactoryFailureWithoutOwnerShowsErrorInPrimaryStage() {
givenVaultState(VaultState.Value.LOCKED);
Mockito.when(unlockWorkflowFactory.create(vault, null)).thenThrow(new RuntimeException("factory failed"));
appWindows.startUnlockWorkflow(vault, null);
runQueuedTasks();
Mockito.verify(errorWindowFactory).create(Mockito.any(), Mockito.eq(primaryStage), Mockito.isNull());
}
@Test
public void testLockFactoryFailureShowsErrorInOwner() {
givenVaultState(VaultState.Value.UNLOCKED);
Mockito.when(lockWorkflowFactory.create(vault, owner)).thenThrow(new RuntimeException("factory failed"));
appWindows.startLockWorkflow(vault, owner);
runQueuedTasks();
Mockito.verify(errorWindowFactory).create(Mockito.any(), Mockito.eq(owner), Mockito.isNull());
}
@Test
public void testLockFactoryFailureWithoutOwnerShowsErrorInPrimaryStage() {
givenVaultState(VaultState.Value.UNLOCKED);
Mockito.when(lockWorkflowFactory.create(vault, null)).thenThrow(new RuntimeException("factory failed"));
appWindows.startLockWorkflow(vault, null);
runQueuedTasks();
Mockito.verify(errorWindowFactory).create(Mockito.any(), Mockito.eq(primaryStage), Mockito.isNull());
}
}
@@ -140,16 +140,6 @@ class UnlockWorkflowTest {
Assertions.assertFalse(vaultSettings.usesReadOnlyMode.get());
}
@Test
public void testExtensionSwitchedOffRetryAbortsWhenVaultIsNotLocked() throws Exception {
var okAction = failWithExtensionSwitchedOffAndCaptureRetry();
Mockito.when(vault.isLocked()).thenReturn(false); // e.g. an unlock from the tray menu started while the dialog was open
okAction.accept(Mockito.mock(Stage.class));
Mockito.verify(appWindows, Mockito.never()).startUnlockWorkflow(Mockito.any(), Mockito.any());
}
private Consumer<Stage> failWithExtensionSwitchedOffAndCaptureRetry() throws Exception {
Mockito.when(dialogs.prepareFSKitExtensionSwitchedOffDialog(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(dialogBuilder);
runFailingWith(new FSKitExtensionSwitchedOffException(new MountFailedException("switched off")));
@@ -179,7 +169,6 @@ class UnlockWorkflowTest {
invocation.getArgument(0, Runnable.class).run();
return null;
});
Mockito.when(vault.isLocked()).thenReturn(true);
Mockito.doThrow(exception).when(keyLoadingStrategy).use(Mockito.any());
workflow.run();