From c306d8df04c343eaeb265ba3582c524c1b3c4db7 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 8 Apr 2021 11:23:57 +0200 Subject: [PATCH] alternative thread-safe vault state without requiring explicit synchronization --- .../common/vaults/DokanyVolume.java | 3 - .../org/cryptomator/common/vaults/Vault.java | 57 +++------ .../common/vaults/VaultComponent.java | 2 +- .../common/vaults/VaultListManager.java | 28 +++-- .../common/vaults/VaultModule.java | 6 - .../cryptomator/common/vaults/VaultState.java | 110 +++++++++++++----- .../cryptomator/common/vaults/VaultStats.java | 6 +- .../cryptomator/ui/common/VaultService.java | 14 +-- .../ui/launcher/AppLifecycleListener.java | 4 +- .../org/cryptomator/ui/lock/LockWorkflow.java | 14 +-- .../ui/mainwindow/VaultDetailController.java | 3 +- .../mainwindow/VaultListCellController.java | 3 +- .../VaultListContextMenuController.java | 11 +- .../ui/migration/MigrationRunController.java | 22 ++-- .../ui/stats/VaultStatisticsModule.java | 4 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 17 +-- 16 files changed, 150 insertions(+), 154 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index e96ed1930..c998761d0 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -13,9 +13,6 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import java.util.concurrent.ExecutorService; import java.util.function.Consumer; public class DokanyVolume extends AbstractVolume { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 584c79456..39cf6d2fd 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -27,7 +27,6 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Provider; -import javafx.application.Platform; import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanBinding; @@ -44,7 +43,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.StampedLock; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -54,13 +52,11 @@ public class Vault { private static final Logger LOG = LoggerFactory.getLogger(Vault.class); private static final Path HOME_DIR = Paths.get(SystemUtils.USER_HOME); - private final StampedLock stateLock; - private final VaultSettings vaultSettings; private final Provider volumeProvider; private final StringBinding defaultMountFlags; private final AtomicReference cryptoFileSystem; - private final ObjectProperty state; + private final VaultState state; private final ObjectProperty lastKnownException; private final VaultStats stats; private final StringBinding displayName; @@ -78,7 +74,7 @@ public class Vault { private volatile Volume volume; @Inject - Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { + Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, VaultState state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { this.vaultSettings = vaultSettings; this.volumeProvider = volumeProvider; this.defaultMountFlags = defaultMountFlags; @@ -97,8 +93,6 @@ public class Vault { this.accessPoint = Bindings.createStringBinding(this::getAccessPoint, state); this.accessPointPresent = this.accessPoint.isNotEmpty(); this.showingStats = new SimpleBooleanProperty(false); - - this.stateLock = new StampedLock(); } // ****************************************************************************** @@ -146,12 +140,9 @@ public class Vault { try { volume = volumeProvider.get(); volume.mount(fs, getEffectiveMountFlags(), throwable -> { + LOG.info("Unmounted vault '{}'", getDisplayName()); destroyCryptoFileSystem(); - new Thread(() -> { //TODO: maybe use the executor service - long stamp = stateLock.writeLock(); - setState(VaultState.LOCKED, stamp); - stateLock.unlock(stamp); - }).start(); + state.set(VaultState.Value.LOCKED); if (throwable != null) { LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); } @@ -182,32 +173,12 @@ public class Vault { // Observable Properties // ******************************************************************************* - public ObjectProperty stateProperty() { + public VaultState stateProperty() { return state; } - public VaultState getState() { - return state.get(); - } - - public long lockVaultState() { - return stateLock.writeLock(); - } - - public void unlockVaultState(long stamp) { - stateLock.unlock(stamp); - } - - public void setState(VaultState value, long stamp) { - if (stateLock.isWriteLockStamp(stamp)) { - if (Platform.isFxApplicationThread()) { - state.setValue(value); - } else { - Platform.runLater(() -> state.setValue(value)); - } - } else { - throw new IllegalCallerException("Stamp is not a valid write lock."); - } + public VaultState.Value getState() { + return state.getValue(); } public ObjectProperty lastKnownExceptionProperty() { @@ -227,7 +198,7 @@ public class Vault { } public boolean isLocked() { - return state.get() == VaultState.LOCKED; + return state.get() == VaultState.Value.LOCKED; } public BooleanBinding processingProperty() { @@ -235,7 +206,7 @@ public class Vault { } public boolean isProcessing() { - return state.get() == VaultState.PROCESSING; + return state.get() == VaultState.Value.PROCESSING; } public BooleanBinding unlockedProperty() { @@ -243,7 +214,7 @@ public class Vault { } public boolean isUnlocked() { - return state.get() == VaultState.UNLOCKED; + return state.get() == VaultState.Value.UNLOCKED; } public BooleanBinding missingProperty() { @@ -251,7 +222,7 @@ public class Vault { } public boolean isMissing() { - return state.get() == VaultState.MISSING; + return state.get() == VaultState.Value.MISSING; } public BooleanBinding needsMigrationProperty() { @@ -259,7 +230,7 @@ public class Vault { } public boolean isNeedsMigration() { - return state.get() == VaultState.NEEDS_MIGRATION; + return state.get() == VaultState.Value.NEEDS_MIGRATION; } public BooleanBinding unknownErrorProperty() { @@ -267,7 +238,7 @@ public class Vault { } public boolean isUnknownError() { - return state.get() == VaultState.ERROR; + return state.get() == VaultState.Value.ERROR; } public StringBinding displayNameProperty() { @@ -283,7 +254,7 @@ public class Vault { } public String getAccessPoint() { - if (state.get() == VaultState.UNLOCKED) { + if (state.getValue() == VaultState.Value.UNLOCKED) { assert volume != null; return volume.getMountPoint().orElse(Path.of("")).toString(); } else { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java index debfab3ed..47be62520 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java @@ -26,7 +26,7 @@ public interface VaultComponent { Builder vaultSettings(VaultSettings vaultSettings); @BindsInstance - Builder initialVaultState(VaultState vaultState); + Builder initialVaultState(VaultState.Value vaultState); @BindsInstance Builder initialErrorCause(@Nullable @Named("lastKnownException") Exception initialErrorCause); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index da9eee66e..4b8ca71e8 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -93,45 +93,43 @@ public class VaultListManager { private Vault create(VaultSettings vaultSettings) { VaultComponent.Builder compBuilder = vaultComponentBuilder.vaultSettings(vaultSettings); try { - VaultState vaultState = determineVaultState(vaultSettings.path().get()); + VaultState.Value vaultState = determineVaultState(vaultSettings.path().get()); compBuilder.initialVaultState(vaultState); } catch (IOException e) { LOG.warn("Failed to determine vault state for " + vaultSettings.path().get(), e); - compBuilder.initialVaultState(VaultState.ERROR); + compBuilder.initialVaultState(VaultState.Value.ERROR); compBuilder.initialErrorCause(e); } return compBuilder.build().vault(); } - public static VaultState redetermineVaultState(Vault vault) { - VaultState previousState = vault.getState(); + public static VaultState.Value redetermineVaultState(Vault vault) { + VaultState state = vault.stateProperty(); + VaultState.Value previousState = state.getValue(); return switch (previousState) { case LOCKED, NEEDS_MIGRATION, MISSING -> { - long stamp = vault.lockVaultState(); try { - VaultState determinedState = determineVaultState(vault.getPath()); - vault.setState(determinedState, stamp); + VaultState.Value determinedState = determineVaultState(vault.getPath()); + state.set(determinedState); yield determinedState; } catch (IOException e) { LOG.warn("Failed to determine vault state for " + vault.getPath(), e); - vault.setState(VaultState.ERROR, stamp); + state.set(VaultState.Value.ERROR); vault.setLastKnownException(e); - yield VaultState.ERROR; - } finally { - vault.unlockVaultState(stamp); + yield VaultState.Value.ERROR; } } case ERROR, UNLOCKED, PROCESSING -> previousState; }; } - private static VaultState determineVaultState(Path pathToVault) throws IOException { + private static VaultState.Value determineVaultState(Path pathToVault) throws IOException { if (!CryptoFileSystemProvider.containsVault(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.MISSING; + return VaultState.Value.MISSING; } else if (Migrators.get().needsMigration(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.NEEDS_MIGRATION; + return VaultState.Value.NEEDS_MIGRATION; } else { - return VaultState.LOCKED; + return VaultState.Value.LOCKED; } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 56a2814cf..b6e070310 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -40,12 +40,6 @@ public class VaultModule { return new AtomicReference<>(); } - @Provides - @PerVault - public ObjectProperty provideVaultState(VaultState initialState) { - return new SimpleObjectProperty<>(initialState); - } - @Provides @Named("lastKnownException") @PerVault diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java index fa5e6c295..8966e6f65 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java @@ -1,34 +1,92 @@ package org.cryptomator.common.vaults; -public enum VaultState { - /** - * No vault found at the provided path - */ - MISSING, +import com.google.common.base.Preconditions; + +import javax.inject.Inject; +import javafx.application.Platform; +import javafx.beans.value.ObservableObjectValue; +import javafx.beans.value.ObservableValueBase; +import java.util.concurrent.atomic.AtomicReference; + +@PerVault +public class VaultState extends ObservableValueBase implements ObservableObjectValue { + + public enum Value { + /** + * No vault found at the provided path + */ + MISSING, + + /** + * Vault requires migration to a newer vault format + */ + NEEDS_MIGRATION, + + /** + * Vault ready to be unlocked + */ + LOCKED, + + /** + * Vault in transition between two other states + */ + PROCESSING, + + /** + * Vault is unlocked + */ + UNLOCKED, + + /** + * Unknown state due to preceeding unrecoverable exceptions. + */ + ERROR; + } + + private final AtomicReference value; + + @Inject + public VaultState(VaultState.Value initialValue){ + this.value = new AtomicReference<>(initialValue); + } + + @Override + public Value get() { + return getValue(); + } + + @Override + public Value getValue() { + return value.get(); + } /** - * Vault requires migration to a newer vault format + * Transitions from fromState to toState. + * @param fromState Previous state + * @param toState New state + * @return true if successful */ - NEEDS_MIGRATION, + public boolean transition(Value fromState, Value toState) { + Preconditions.checkArgument(fromState != toState, "fromState must be different than toState"); + boolean success = value.compareAndSet(fromState, toState); + if (success) { + fireValueChangedEvent(); + } + return success; + } - /** - * Vault ready to be unlocked - */ - LOCKED, - - /** - * Vault in transition between two other states - */ - PROCESSING, - - /** - * Vault is unlocked - */ - UNLOCKED, - - /** - * Unknown state due to preceeding unrecoverable exceptions. - */ - ERROR; + public void set(Value newState) { + var oldState = value.getAndSet(newState); + if (oldState != newState) { + fireValueChangedEvent(); + } + } + protected void fireValueChangedEvent() { + if (Platform.isFxApplicationThread()) { + super.fireValueChangedEvent(); + } else { + Platform.runLater(super::fireValueChangedEvent); + } + } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java index 90edd6604..6dc86e8be 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java @@ -26,7 +26,7 @@ public class VaultStats { private static final Logger LOG = LoggerFactory.getLogger(VaultStats.class); private final AtomicReference fs; - private final ObjectProperty state; + private final VaultState state; private final ScheduledService> updateService; private final LongProperty bytesPerSecondRead = new SimpleLongProperty(); private final LongProperty bytesPerSecondWritten = new SimpleLongProperty(); @@ -41,7 +41,7 @@ public class VaultStats { private final LongProperty filesWritten = new SimpleLongProperty(); @Inject - VaultStats(AtomicReference fs, ObjectProperty state, ExecutorService executor) { + VaultStats(AtomicReference fs, VaultState state, ExecutorService executor) { this.fs = fs; this.state = state; this.updateService = new UpdateStatsService(); @@ -52,7 +52,7 @@ public class VaultStats { } private void vaultStateChanged(@SuppressWarnings("unused") Observable observable) { - if (VaultState.UNLOCKED == state.get()) { + if (VaultState.Value.UNLOCKED == state.get()) { assert fs.get() != null; LOG.debug("start recording stats"); Platform.runLater(() -> updateService.restart()); diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java index d96bd6ae2..6555904e5 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java @@ -163,8 +163,6 @@ public class VaultService { private final Vault vault; private final boolean forced; - private volatile long stamp; - /** * @param vault The vault to lock * @param forced Whether to attempt a forced lock @@ -178,32 +176,28 @@ public class VaultService { @Override protected Vault call() throws Volume.VolumeException { - this.stamp = vault.lockVaultState(); vault.lock(forced); return vault; } @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void failed() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } @Override protected void cancelled() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java index 646c3ce6b..5e2ed383a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java @@ -24,11 +24,13 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import static org.cryptomator.common.vaults.VaultState.Value.*; + @Singleton public class AppLifecycleListener { private static final Logger LOG = LoggerFactory.getLogger(AppLifecycleListener.class); - public static final Set STATES_ALLOWING_TERMINATION = EnumSet.of(VaultState.LOCKED, VaultState.NEEDS_MIGRATION, VaultState.MISSING, VaultState.ERROR); + public static final Set STATES_ALLOWING_TERMINATION = EnumSet.of(LOCKED, NEEDS_MIGRATION, MISSING, ERROR); private final FxApplicationStarter fxApplicationStarter; private final CountDownLatch shutdownLatch; diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index e7f0eb025..673b74641 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -36,8 +36,6 @@ public class LockWorkflow extends Task { private final Lazy lockForcedScene; private final Lazy lockFailedScene; - private volatile long stamp; - @Inject public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene) { this.lockWindow = lockWindow; @@ -49,7 +47,6 @@ public class LockWorkflow extends Task { @Override protected Void call() throws Volume.VolumeException, InterruptedException { - this.stamp = vault.lockVaultState(); try { vault.lock(false); } catch (Volume.VolumeException e) { @@ -82,21 +79,19 @@ public class LockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { LOG.info("Lock of {} succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void failed() { LOG.warn("Failed to lock {}.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); lockWindow.setScene(lockFailedScene.get()); lockWindow.show(); } @@ -104,8 +99,7 @@ public class LockWorkflow extends Task { @Override protected void cancelled() { LOG.debug("Lock of {} canceled.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java index 40df6b7c5..7b5c4c7c9 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java @@ -32,7 +32,8 @@ public class VaultDetailController implements FxController { this.anyVaultSelected = vault.isNotNull(); } - private FontAwesome5Icon getGlyphForVaultState(VaultState state) { + // TODO deduplicate w/ VaultListCellController + private FontAwesome5Icon getGlyphForVaultState(VaultState.Value state) { if (state != null) { return switch (state) { case LOCKED -> FontAwesome5Icon.LOCK; diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java index abc287d6e..f18369e7a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java @@ -24,7 +24,8 @@ public class VaultListCellController implements FxController { .map(this::getGlyphForVaultState); } - private FontAwesome5Icon getGlyphForVaultState(VaultState state) { + // TODO deduplicate w/ VaultDetailController + private FontAwesome5Icon getGlyphForVaultState(VaultState.Value state) { if (state != null) { return switch (state) { case LOCKED -> FontAwesome5Icon.LOCK; diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java index b0866d80f..145618fa8 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java @@ -14,19 +14,13 @@ import org.cryptomator.ui.vaultoptions.VaultOptionsComponent; import javax.inject.Inject; import javafx.beans.binding.Binding; -import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; import javafx.fxml.FXML; import javafx.stage.Stage; -import java.util.Arrays; import java.util.EnumSet; import java.util.Optional; -import static org.cryptomator.common.vaults.VaultState.ERROR; -import static org.cryptomator.common.vaults.VaultState.LOCKED; -import static org.cryptomator.common.vaults.VaultState.MISSING; -import static org.cryptomator.common.vaults.VaultState.NEEDS_MIGRATION; -import static org.cryptomator.common.vaults.VaultState.UNLOCKED; +import static org.cryptomator.common.vaults.VaultState.Value.*; @MainWindowScoped public class VaultListContextMenuController implements FxController { @@ -37,7 +31,7 @@ public class VaultListContextMenuController implements FxController { private final KeychainManager keychain; private final RemoveVaultComponent.Builder removeVault; private final VaultOptionsComponent.Builder vaultOptionsWindow; - private final OptionalBinding selectedVaultState; + private final OptionalBinding selectedVaultState; private final Binding selectedVaultPassphraseStored; private final Binding selectedVaultRemovable; private final Binding selectedVaultUnlockable; @@ -57,7 +51,6 @@ public class VaultListContextMenuController implements FxController { this.selectedVaultRemovable = selectedVaultState.map(EnumSet.of(LOCKED, MISSING, ERROR, NEEDS_MIGRATION)::contains).orElse(false); this.selectedVaultUnlockable = selectedVaultState.map(LOCKED::equals).orElse(false); this.selectedVaultLockable = selectedVaultState.map(UNLOCKED::equals).orElse(false); - } private boolean isPasswordStored(Vault vault) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java index 75caddf5d..0fc66c992 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java @@ -26,6 +26,7 @@ import javax.inject.Named; import javafx.application.Platform; import javafx.beans.binding.Bindings; import javafx.beans.binding.ObjectBinding; +import javafx.beans.binding.ObjectExpression; import javafx.beans.property.BooleanProperty; import javafx.beans.property.DoubleProperty; import javafx.beans.property.ObjectProperty; @@ -89,7 +90,10 @@ public class MigrationRunController implements FxController { if (keychain.isSupported()) { loadStoredPassword(); } - migrationButtonDisabled.bind(vault.stateProperty().isNotEqualTo(VaultState.NEEDS_MIGRATION).or(passwordField.textProperty().isEmpty())); + + migrationButtonDisabled.bind(ObjectExpression.objectExpression(vault.stateProperty()) + .isNotEqualTo(VaultState.Value.NEEDS_MIGRATION) + .or(passwordField.textProperty().isEmpty())); } @FXML @@ -101,8 +105,7 @@ public class MigrationRunController implements FxController { public void migrate() { LOG.info("Migrating vault {}", vault.getPath()); CharSequence password = passwordField.getCharacters(); - long stamp = vault.lockVaultState(); - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.NEEDS_MIGRATION, VaultState.Value.PROCESSING); passwordField.setDisable(true); ScheduledFuture progressSyncTask = scheduler.scheduleAtFixedRate(() -> { Platform.runLater(() -> { @@ -116,10 +119,10 @@ public class MigrationRunController implements FxController { }).onSuccess(needsAnotherMigration -> { if (needsAnotherMigration) { LOG.info("Migration of '{}' succeeded, but another migration is required.", vault.getDisplayName()); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); } else { LOG.info("Migration of '{}' succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); passwordField.wipe(); window.setScene(successScene.get()); } @@ -128,23 +131,22 @@ public class MigrationRunController implements FxController { passwordField.setDisable(false); passwordField.selectAll(); passwordField.requestFocus(); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); }).onError(FileSystemCapabilityChecker.MissingCapabilityException.class, e -> { LOG.error("Underlying file system not supported.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); missingCapability.set(e.getMissingCapability()); window.setScene(capabilityErrorScene.get()); }).onError(FileNameTooLongException.class, e -> { LOG.error("Migration failed because the underlying file system does not support long filenames.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); window.setScene(impossibleScene.get()); }).onError(Exception.class, e -> { // including RuntimeExceptions LOG.error("Migration failed for technical reasons.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); }).andFinally(() -> { - vault.unlockVaultState(stamp); passwordField.setDisable(false); progressSyncTask.cancel(true); }).runOnce(executor); diff --git a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java index 803de314c..6f195274c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java @@ -43,8 +43,8 @@ abstract class VaultStatisticsModule { var weakStage = new WeakReference<>(stage); vault.stateProperty().addListener(new ChangeListener<>() { @Override - public void changed(ObservableValue observable, VaultState oldValue, VaultState newValue) { - if (newValue != VaultState.UNLOCKED) { + public void changed(ObservableValue observable, VaultState.Value oldValue, VaultState.Value newValue) { + if (newValue != VaultState.Value.UNLOCKED) { Stage stage = weakStage.get(); if (stage != null) { stage.hide(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index 36d34c170..08ae1fd4c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -59,8 +59,6 @@ public class UnlockWorkflow extends Task { private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; - private volatile long stamp; - @Inject UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { this.window = window; @@ -89,7 +87,6 @@ public class UnlockWorkflow extends Task { @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { try { - this.stamp = vault.lockVaultState(); if (attemptUnlock()) { handleSuccess(); return true; @@ -176,15 +173,12 @@ public class UnlockWorkflow extends Task { LOG.error("Unlock failed. Mountpoint doesn't exist (needs to be a folder): {}", cause.getMessage()); } showInvalidMountPointScene(); - return; } else if (cause instanceof FileAlreadyExistsException) { LOG.error("Unlock failed. Mountpoint already exists: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else if (cause instanceof DirectoryNotEmptyException) { LOG.error("Unlock failed. Mountpoint not an empty directory: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else { handleGenericError(impExc); } @@ -210,25 +204,22 @@ public class UnlockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } @Override protected void failed() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void cancelled() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } }