more robust settings, exposed saveNow()

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<Path> getPluginDir() {
 		return getPath(PLUGIN_DIR_PROP_NAME);
 	}

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<VaultSettings> directories;
 	public final BooleanProperty startHidden;
 	public final BooleanProperty autoCloseVaults;
@@ -79,12 +78,10 @@ public class Settings {
 	public final ObjectProperty<Instant> lastSuccessfulUpdateCheck;
 	public final ObjectProperty<Path> previouslyUsedVaultDirectory;
 
-	private Consumer<Settings> 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<Settings> 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);
 	}
 
 }

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 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<Settings> {
 		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<Settings> {
 		}
 	}
 
-	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<Path> 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<Settings> {
 			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);

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<Settings> 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);
 	}
 
 }