From 8a434dcd96b849ac38fb3602a3f032135808bd0b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 5 Nov 2025 22:23:34 +0100 Subject: [PATCH] more robust settings, exposed `saveNow()` --- .../org/cryptomator/common/Environment.java | 9 ++++ .../cryptomator/common/settings/Settings.java | 26 +++------- .../common/settings/SettingsProvider.java | 49 ++++++++++++------- .../common/settings/SettingsTest.java | 15 +++--- 4 files changed, 55 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/cryptomator/common/Environment.java b/src/main/java/org/cryptomator/common/Environment.java index 39214e626..0a71b28a9 100644 --- a/src/main/java/org/cryptomator/common/Environment.java +++ b/src/main/java/org/cryptomator/common/Environment.java @@ -124,6 +124,15 @@ public class Environment { return Optional.ofNullable(System.getProperty(BUILD_NUMBER_PROP_NAME)); } + /** + * Returns the app version concatenated with the build number (if defined). + * + * @return version string formatted like {@code 1.2.3-4567} or {@code 1.2.3} if no build number is defined. + */ + public String getAppVersionWithBuildNumber() { + return getAppVersion() + getBuildNumber().map("-"::concat).orElse(""); + } + public Optional getPluginDir() { return getPath(PLUGIN_DIR_PROP_NAME); } diff --git a/src/main/java/org/cryptomator/common/settings/Settings.java b/src/main/java/org/cryptomator/common/settings/Settings.java index a711e6536..45b6bc032 100644 --- a/src/main/java/org/cryptomator/common/settings/Settings.java +++ b/src/main/java/org/cryptomator/common/settings/Settings.java @@ -25,10 +25,8 @@ import javafx.beans.property.StringProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.geometry.NodeOrientation; - import java.nio.file.Path; import java.time.Instant; -import java.util.function.Consumer; public class Settings { @@ -53,6 +51,7 @@ public class Settings { static final String DEFAULT_USER_INTERFACE_ORIENTATION = NodeOrientation.LEFT_TO_RIGHT.name(); public static final Instant DEFAULT_TIMESTAMP = Instant.parse("2000-01-01T00:00:00Z"); + private final SettingsProvider provider; public final ObservableList directories; public final BooleanProperty startHidden; public final BooleanProperty autoCloseVaults; @@ -79,12 +78,10 @@ public class Settings { public final ObjectProperty lastSuccessfulUpdateCheck; public final ObjectProperty previouslyUsedVaultDirectory; - private Consumer saveCmd; - - public static Settings create(Environment env) { + public static Settings create(SettingsProvider provider, Environment env) { var defaults = new SettingsJson(); defaults.showTrayIcon = env.showTrayIcon(); - return new Settings(defaults); + return new Settings(provider, defaults); } /** @@ -92,7 +89,8 @@ public class Settings { * * @param json The parsed settings.json */ - Settings(SettingsJson json) { + Settings(SettingsProvider provider, SettingsJson json) { + this.provider = provider; this.directories = FXCollections.observableArrayList(VaultSettings::observables); this.startHidden = new SimpleBooleanProperty(this, "startHidden", json.startHidden); this.autoCloseVaults = new SimpleBooleanProperty(this, "autoCloseVaults", json.autoCloseVaults); @@ -222,20 +220,12 @@ public class Settings { } } - - // TODO rename to setChangeListener - void setSaveCmd(Consumer saveCmd) { - this.saveCmd = saveCmd; - } - private void somethingChanged(@SuppressWarnings("unused") Observable observable) { - this.save(); + provider.scheduleSave(this); } - void save() { - if (saveCmd != null) { - saveCmd.accept(this); - } + public void saveNow() { + provider.saveNow(this); } } diff --git a/src/main/java/org/cryptomator/common/settings/SettingsProvider.java b/src/main/java/org/cryptomator/common/settings/SettingsProvider.java index d9fab7108..0a4a0f190 100644 --- a/src/main/java/org/cryptomator/common/settings/SettingsProvider.java +++ b/src/main/java/org/cryptomator/common/settings/SettingsProvider.java @@ -26,7 +26,9 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; -import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -61,8 +63,7 @@ public class SettingsProvider implements Supplier { Settings settings = env.getSettingsPath() // .flatMap(this::tryLoad) // .findFirst() // - .orElseGet(() -> Settings.create(env)); - settings.setSaveCmd(this::scheduleSave); + .orElseGet(() -> Settings.create(this, env)); return settings; } @@ -71,7 +72,7 @@ public class SettingsProvider implements Supplier { try (InputStream in = Files.newInputStream(path, StandardOpenOption.READ)) { var json = JSON.reader().readValue(in, SettingsJson.class); LOG.info("Settings loaded from {}", path); - var settings = new Settings(json); + var settings = new Settings(this, json); return Stream.of(settings); } catch (JacksonException e) { LOG.warn("Failed to parse json file {}", path, e); @@ -84,19 +85,33 @@ public class SettingsProvider implements Supplier { } } - private void scheduleSave(Settings settings) { - if (settings == null) { - return; + void saveNow(Settings settings) { + try { + scheduleSave(settings, 0L).get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + LOG.error("Saving settings was interrupted.", e); + } catch (ExecutionException e) { + LOG.error("Unexpected exception while saving.", e); } - final Optional settingsPath = env.getSettingsPath().findFirst(); // always save to preferred (first) path - settingsPath.ifPresent(path -> { - Runnable saveCommand = () -> this.save(settings, path); - ScheduledFuture scheduledTask = scheduler.schedule(saveCommand, SAVE_DELAY_MS, TimeUnit.MILLISECONDS); - ScheduledFuture previouslyScheduledTask = scheduledSaveCmd.getAndSet(scheduledTask); - if (previouslyScheduledTask != null) { - previouslyScheduledTask.cancel(false); - } - }); + } + + void scheduleSave(Settings settings) { + scheduleSave(settings, SAVE_DELAY_MS); + } + + private Future scheduleSave(Settings settings, long delayMillis) { + if (settings == null) { + return CompletableFuture.completedFuture(null); + } + final Path settingsPath = env.getSettingsPath().findFirst().orElseThrow(); // always save to preferred (first) path + Runnable saveCommand = () -> this.save(settings, settingsPath); + ScheduledFuture scheduledTask = scheduler.schedule(saveCommand, delayMillis, TimeUnit.MILLISECONDS); + ScheduledFuture previouslyScheduledTask = scheduledSaveCmd.getAndSet(scheduledTask); + if (previouslyScheduledTask != null) { + previouslyScheduledTask.cancel(false); + } + return scheduledTask; } private void save(Settings settings, Path settingsPath) { @@ -107,7 +122,7 @@ public class SettingsProvider implements Supplier { Path tmpPath = settingsPath.resolveSibling(settingsPath.getFileName().toString() + ".tmp"); try (OutputStream out = Files.newOutputStream(tmpPath, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE)) { var jsonObj = settings.serialized(); - jsonObj.writtenByVersion = env.getAppVersion() + env.getBuildNumber().map("-"::concat).orElse(""); + jsonObj.writtenByVersion = env.getAppVersionWithBuildNumber(); JSON.writerWithDefaultPrettyPrinter().writeValue(out, jsonObj); } Files.move(tmpPath, settingsPath, StandardCopyOption.REPLACE_EXISTING); diff --git a/src/test/java/org/cryptomator/common/settings/SettingsTest.java b/src/test/java/org/cryptomator/common/settings/SettingsTest.java index cd737ac11..a7ccf2e7a 100644 --- a/src/test/java/org/cryptomator/common/settings/SettingsTest.java +++ b/src/test/java/org/cryptomator/common/settings/SettingsTest.java @@ -9,31 +9,28 @@ import org.cryptomator.common.Environment; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.util.function.Consumer; - public class SettingsTest { @Test public void testAutoSave() { Environment env = Mockito.mock(Environment.class); - @SuppressWarnings("unchecked") Consumer changeListener = Mockito.mock(Consumer.class); + SettingsProvider provider = Mockito.mock(SettingsProvider.class); - Settings settings = Settings.create(env); - settings.setSaveCmd(changeListener); + Settings settings = Settings.create(provider, env); VaultSettings vaultSettings = VaultSettings.withRandomId(); - Mockito.verify(changeListener, Mockito.times(0)).accept(settings); + Mockito.verify(provider, Mockito.times(0)).scheduleSave(settings); // first change (to property): settings.port.set(42428); - Mockito.verify(changeListener, Mockito.times(1)).accept(settings); + Mockito.verify(provider, Mockito.times(1)).scheduleSave(settings); // second change (to list): settings.directories.add(vaultSettings); - Mockito.verify(changeListener, Mockito.times(2)).accept(settings); + Mockito.verify(provider, Mockito.times(2)).scheduleSave(settings); // third change (to property of list item): vaultSettings.displayName.set("asd"); - Mockito.verify(changeListener, Mockito.times(3)).accept(settings); + Mockito.verify(provider, Mockito.times(3)).scheduleSave(settings); } }