Keep an unlock admitted while the tray refreshes vault states, and write vault states only by transition

This commit is contained in:
Tobias Hagemann
2026-10-14 15:37:52 +02:00
parent b82993ae3c
commit 8f344ebd09
5 changed files with 95 additions and 14 deletions
+1
View File
@@ -17,6 +17,7 @@ Changes to prior versions can be found on the [Github release page](https://gith
* Fixed password retrieval from keychain not working while the system keychain is locked on Linux ([#4294](https://github.com/cryptomator/cryptomator/pull/4294), [#4301](https://github.com/cryptomator/cryptomator/pull/4301), [#4302](https://github.com/cryptomator/cryptomator/pull/4302))
* 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
### Changed
* Improved usability of the trust on first use dialog ([#4295](https://github.com/cryptomator/cryptomator/pull/4295))
@@ -237,16 +237,23 @@ public class VaultListManager {
vault.getVaultConfigCache().reloadConfig();
}
state.set(determined);
replaceUnlessChanged(state, previous, determined);
return determined;
} catch (IOException e) {
LOG.warn("Failed to (re)determine vault state for {}", vault.getPath(), e);
vault.setLastKnownException(e);
state.set(ERROR);
replaceUnlessChanged(state, previous, ERROR);
return ERROR;
}
}
// a workflow may have taken the vault into PROCESSING while its state was being determined
private static void replaceUnlessChanged(VaultState state, VaultState.Value previous, VaultState.Value next) {
if (next != previous) {
state.transition(previous, next);
}
}
public static VaultState.Value determineVaultState(Path pathToVault) throws IOException {
if (!Files.exists(pathToVault)) {
return MISSING;
@@ -98,13 +98,6 @@ public class VaultState extends ObservableValueBase<VaultState.Value> implements
return success;
}
public void set(Value newState) {
var oldState = value.getAndSet(newState);
if (oldState != newState) {
fireValueChangedEvent();
}
}
/**
* Waits for the specified time, until the desired state is reached.
*
@@ -2,7 +2,9 @@ package org.cryptomator.common.vaults;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mockito;
import javafx.application.Platform;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -67,4 +69,82 @@ class VaultListManagerTest {
});
}
@Test
void testRedetermineVaultStateUpdatesState(@TempDir Path tmpDir) {
var state = new VaultState(VaultState.Value.LOCKED);
var vault = Mockito.mock(Vault.class);
Mockito.when(vault.stateProperty()).thenReturn(state);
Mockito.when(vault.getPath()).thenReturn(tmpDir.resolve("missing"));
try (var _ = Mockito.mockStatic(Platform.class)) {
VaultListManager.redetermineVaultState(vault);
}
assertEquals(VaultState.Value.MISSING, state.getValue());
}
@Test
void testRedetermineVaultStateKeepsUnchangedState(@TempDir Path tmpDir) {
var state = new VaultState(VaultState.Value.MISSING);
var vault = Mockito.mock(Vault.class);
Mockito.when(vault.stateProperty()).thenReturn(state);
Mockito.when(vault.getPath()).thenReturn(tmpDir.resolve("missing"));
try (var _ = Mockito.mockStatic(Platform.class)) {
assertEquals(VaultState.Value.MISSING, VaultListManager.redetermineVaultState(vault));
}
assertEquals(VaultState.Value.MISSING, state.getValue());
}
@Test
void testRedetermineVaultStateKeepsStateAdmittedMeanwhile(@TempDir Path tmpDir) {
var state = new VaultState(VaultState.Value.LOCKED);
var vault = Mockito.mock(Vault.class);
Mockito.when(vault.stateProperty()).thenReturn(state);
Mockito.when(vault.getPath()).thenAnswer(_ -> {
state.transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING); // an unlock is admitted during the check
return tmpDir.resolve("missing");
});
try (var _ = Mockito.mockStatic(Platform.class)) {
VaultListManager.redetermineVaultState(vault);
}
assertEquals(VaultState.Value.PROCESSING, state.getValue());
}
@Test
void testRedetermineVaultStateMarksError(@TempDir Path tmpDir) throws IOException {
var notADirectory = Files.createFile(tmpDir.resolve("file"));
var state = new VaultState(VaultState.Value.LOCKED);
var vault = Mockito.mock(Vault.class);
Mockito.when(vault.stateProperty()).thenReturn(state);
Mockito.when(vault.getPath()).thenReturn(notADirectory);
try (var _ = Mockito.mockStatic(Platform.class)) {
VaultListManager.redetermineVaultState(vault);
}
assertEquals(VaultState.Value.ERROR, state.getValue());
}
@Test
void testRedetermineVaultStateKeepsStateAdmittedMeanwhileOnError(@TempDir Path tmpDir) throws IOException {
var notADirectory = Files.createFile(tmpDir.resolve("file"));
var state = new VaultState(VaultState.Value.LOCKED);
var vault = Mockito.mock(Vault.class);
Mockito.when(vault.stateProperty()).thenReturn(state);
Mockito.when(vault.getPath()).thenAnswer(_ -> {
state.transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING); // an unlock is admitted during the check
return notADirectory;
});
try (var _ = Mockito.mockStatic(Platform.class)) {
VaultListManager.redetermineVaultState(vault);
}
assertEquals(VaultState.Value.PROCESSING, state.getValue());
}
}
@@ -113,7 +113,7 @@ class VaultTest {
@Test
public void testTerminationDuringProcessingLeavesStateToOwner() throws Exception {
unlock(mount, true);
state.set(VaultState.Value.PROCESSING);
state.transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
terminated.complete(null);
@@ -127,7 +127,7 @@ class VaultTest {
@Test
public void testTerminationWhileForceDecisionPendingLocks() throws Exception {
unlock(mount, true, true);
state.set(VaultState.Value.PROCESSING);
state.transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
Mockito.doThrow(UnmountFailedException.class).when(mount).unmount();
Assertions.assertThrows(UnmountFailedException.class, () -> vault.lock(false));
@@ -214,7 +214,7 @@ class VaultTest {
@Test
public void testFinishProcessingWithMountSettlesUnlocked() throws Exception {
unlock(mount, false);
state.set(VaultState.Value.PROCESSING);
state.transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
vault.finishProcessing();
@@ -224,7 +224,7 @@ class VaultTest {
@Test
public void testFinishProcessingAfterFailedCloseSettlesLocked() throws Exception {
unlock(mount, false);
state.set(VaultState.Value.PROCESSING);
state.transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
Mockito.doThrow(IOException.class).when(mount).close();
Assertions.assertThrows(IOException.class, () -> vault.lock(false));
@@ -244,7 +244,7 @@ class VaultTest {
public void testMarkLockedIfUnmountedLeavesProcessing() throws Exception {
unlock(mount, false);
vault.lock(false);
state.set(VaultState.Value.PROCESSING);
state.transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING);
Assertions.assertFalse(vault.markLockedIfUnmounted());
Assertions.assertEquals(VaultState.Value.PROCESSING, state.getValue());