From d7231571c666e9a1cc08ca5d9d823782785f2040 Mon Sep 17 00:00:00 2001 From: Tobias Nett Date: Fri, 16 Oct 2020 18:40:12 +0200 Subject: [PATCH] refactoring: Streamline LauncherSettings to ease replacement (#590) --- .../launcher/LauncherConfiguration.java | 21 ++------ .../terasology/launcher/LauncherInitTask.java | 37 ++++++------- .../launcher/NullLauncherConfiguration.java | 2 +- .../launcher/TerasologyLauncher.java | 9 +++- .../gui/javafx/ApplicationController.java | 9 ++-- .../gui/javafx/SettingsController.java | 12 +++-- .../settings/BaseLauncherSettings.java | 44 ++------------- ...herSettings.java => LauncherSettings.java} | 9 ++-- .../settings/LauncherSettingsValidator.java | 4 +- .../launcher/settings/Settings.java | 54 +++++++++++++++++++ .../settings/SettingsValidationRule.java | 12 ++--- ...ettings.java => TestLauncherSettings.java} | 10 ++-- 12 files changed, 121 insertions(+), 102 deletions(-) rename src/main/java/org/terasology/launcher/settings/{AbstractLauncherSettings.java => LauncherSettings.java} (96%) create mode 100644 src/main/java/org/terasology/launcher/settings/Settings.java rename src/test/java/org/terasology/launcher/settings/{TestBaseLauncherSettings.java => TestLauncherSettings.java} (96%) diff --git a/src/main/java/org/terasology/launcher/LauncherConfiguration.java b/src/main/java/org/terasology/launcher/LauncherConfiguration.java index d6292c462..a3a260d46 100644 --- a/src/main/java/org/terasology/launcher/LauncherConfiguration.java +++ b/src/main/java/org/terasology/launcher/LauncherConfiguration.java @@ -18,6 +18,7 @@ import org.terasology.launcher.packages.PackageManager; import org.terasology.launcher.settings.BaseLauncherSettings; +import org.terasology.launcher.settings.LauncherSettings; import java.nio.file.Path; @@ -36,21 +37,15 @@ public class LauncherConfiguration { private final Path launcherDirectory; private final Path downloadDirectory; - private final Path tempDirectory; - private final Path cacheDirectory; - private final BaseLauncherSettings launcherSettings; + private final LauncherSettings launcherSettings; private final PackageManager packageManager; public LauncherConfiguration(final Path launcherDirectory, final Path downloadDirectory, - final Path tempDirectory, - final Path cacheDirectory, - final BaseLauncherSettings launcherSettings, + final LauncherSettings launcherSettings, final PackageManager packageManager) { this.launcherDirectory = launcherDirectory; this.downloadDirectory = downloadDirectory; - this.tempDirectory = tempDirectory; - this.cacheDirectory = cacheDirectory; this.launcherSettings = launcherSettings; this.packageManager = packageManager; } @@ -63,15 +58,7 @@ public Path getDownloadDirectory() { return downloadDirectory; } - public Path getTempDirectory() { - return tempDirectory; - } - - public Path getCacheDirectory() { - return cacheDirectory; - } - - public BaseLauncherSettings getLauncherSettings() { + public LauncherSettings getLauncherSettings() { return launcherSettings; } diff --git a/src/main/java/org/terasology/launcher/LauncherInitTask.java b/src/main/java/org/terasology/launcher/LauncherInitTask.java index 46f4458ef..cfc977d82 100644 --- a/src/main/java/org/terasology/launcher/LauncherInitTask.java +++ b/src/main/java/org/terasology/launcher/LauncherInitTask.java @@ -26,7 +26,9 @@ import org.slf4j.LoggerFactory; import org.terasology.launcher.packages.PackageManager; import org.terasology.launcher.settings.BaseLauncherSettings; +import org.terasology.launcher.settings.LauncherSettings; import org.terasology.launcher.settings.LauncherSettingsValidator; +import org.terasology.launcher.settings.Settings; import org.terasology.launcher.updater.LauncherUpdater; import org.terasology.launcher.util.BundleUtils; import org.terasology.launcher.util.DirectoryCreator; @@ -44,6 +46,7 @@ import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; import java.util.concurrent.CompletableFuture; public class LauncherInitTask extends Task { @@ -81,7 +84,8 @@ protected LauncherConfiguration call() { final Path cacheDirectory = getDirectoryFor(LauncherManagedDirectory.CACHE, userDataDirectory); // launcher settings - final BaseLauncherSettings launcherSettings = getLauncherSettings(userDataDirectory); + final Path settingsFile = userDataDirectory.resolve(Settings.DEFAULT_FILE_NAME); + final LauncherSettings launcherSettings = getLauncherSettings(settingsFile); // validate the settings LauncherSettingsValidator.validate(launcherSettings); @@ -119,11 +123,11 @@ protected LauncherConfiguration call() { launcherSettings.setGameDataDirectory(gameDataDirectory); // TODO: Rewrite gameVersions.fixSettingsBuildVersion(launcherSettings); - storeLauncherSettingsAfterInit(launcherSettings); + storeLauncherSettingsAfterInit(launcherSettings, settingsFile); logger.trace("Creating launcher frame..."); - return new LauncherConfiguration(userDataDirectory, downloadDirectory, tempDirectory, cacheDirectory, launcherSettings, packageManager); + return new LauncherConfiguration(userDataDirectory, downloadDirectory, launcherSettings, packageManager); } catch (LauncherStartFailedException e) { logger.warn("Could not configure launcher."); } @@ -172,20 +176,16 @@ private Path getLauncherDirectory(Platform platform) throws LauncherStartFailedE return launcherDirectory; } - private BaseLauncherSettings getLauncherSettings(Path launcherDirectory) throws LauncherStartFailedException { + private LauncherSettings getLauncherSettings(Path settingsFile) throws LauncherStartFailedException { logger.trace("Init LauncherSettings..."); updateMessage(BundleUtils.getLabel("splash_retrieveLauncherSettings")); - final BaseLauncherSettings launcherSettings = new BaseLauncherSettings(launcherDirectory); - try { - launcherSettings.load(); - launcherSettings.init(); - } catch (IOException e) { - logger.error("The launcher settings can not be loaded or initialized! '{}'", launcherSettings.getLauncherSettingsFilePath(), e); - Dialogs.showError(owner, BundleUtils.getLabel("message_error_loadSettings") + "\n" + launcherSettings.getLauncherSettingsFilePath()); - throw new LauncherStartFailedException(); - } - logger.debug("Launcher Settings: {}", launcherSettings); - return launcherSettings; + + final LauncherSettings settings = Optional.ofNullable(Settings.load(settingsFile)).orElse(Settings.getDefault()); + settings.init(); + + logger.debug("Launcher Settings: {}", settings); + + return settings; } private boolean checkForLauncherUpdates(Path downloadDirectory, Path tempDirectory, boolean saveDownloadedFiles) { @@ -289,14 +289,15 @@ private boolean confirmSourcesOverwrite() { }, javafx.application.Platform::runLater).join(); } - private void storeLauncherSettingsAfterInit(BaseLauncherSettings launcherSettings) throws LauncherStartFailedException { + private void storeLauncherSettingsAfterInit(LauncherSettings launcherSettings, final Path settingsFile) throws LauncherStartFailedException { logger.trace("Store LauncherSettings..."); updateMessage(BundleUtils.getLabel("splash_storeLauncherSettings")); try { - launcherSettings.store(); + Settings.store(launcherSettings, settingsFile); } catch (IOException e) { - logger.error("The launcher settings can not be stored! '{}'", launcherSettings.getLauncherSettingsFilePath(), e); + logger.error("The launcher settings cannot be stored! '{}'", settingsFile, e); Dialogs.showError(owner, BundleUtils.getLabel("message_error_storeSettings")); + //TODO: should we fail here, or is it fine to work with in-memory settings? throw new LauncherStartFailedException(); } logger.debug("Launcher Settings stored: {}", launcherSettings); diff --git a/src/main/java/org/terasology/launcher/NullLauncherConfiguration.java b/src/main/java/org/terasology/launcher/NullLauncherConfiguration.java index 897b64279..ef233aec5 100644 --- a/src/main/java/org/terasology/launcher/NullLauncherConfiguration.java +++ b/src/main/java/org/terasology/launcher/NullLauncherConfiguration.java @@ -28,7 +28,7 @@ public final class NullLauncherConfiguration extends LauncherConfiguration { private static NullLauncherConfiguration instance; private NullLauncherConfiguration() { - super(null, null, null, null, null, null); + super(null, null, null, null); } public static NullLauncherConfiguration getInstance() { diff --git a/src/main/java/org/terasology/launcher/TerasologyLauncher.java b/src/main/java/org/terasology/launcher/TerasologyLauncher.java index 8b64f0482..16dc78508 100644 --- a/src/main/java/org/terasology/launcher/TerasologyLauncher.java +++ b/src/main/java/org/terasology/launcher/TerasologyLauncher.java @@ -150,8 +150,13 @@ private void showMainStage(final LauncherConfiguration launcherConfiguration) th root = (Parent) fxmlLoader.load(); } final ApplicationController controller = fxmlLoader.getController(); - controller.update(launcherConfiguration.getLauncherDirectory(), launcherConfiguration.getDownloadDirectory(), - launcherConfiguration.getLauncherSettings(), launcherConfiguration.getPackageManager(), mainStage, hostServices); + controller.update( + launcherConfiguration.getLauncherDirectory(), + launcherConfiguration.getDownloadDirectory(), + launcherConfiguration.getLauncherSettings(), + launcherConfiguration.getPackageManager(), + mainStage, + hostServices); Scene scene = new Scene(root); scene.getStylesheets().add(BundleUtils.getStylesheet("css_terasology")); diff --git a/src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java b/src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java index c79e9ead4..cbd389dd8 100644 --- a/src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java +++ b/src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java @@ -50,6 +50,8 @@ import org.terasology.launcher.packages.Package; import org.terasology.launcher.packages.PackageManager; import org.terasology.launcher.settings.BaseLauncherSettings; +import org.terasology.launcher.settings.LauncherSettings; +import org.terasology.launcher.settings.Settings; import org.terasology.launcher.tasks.DeleteTask; import org.terasology.launcher.tasks.DownloadTask; import org.terasology.launcher.util.BundleUtils; @@ -73,7 +75,7 @@ public class ApplicationController { private static final long MINIMUM_FREE_SPACE = 200 * MB; private Path launcherDirectory; - private BaseLauncherSettings launcherSettings; + private LauncherSettings launcherSettings; private PackageManager packageManager; private GameStarter gameStarter; private Stage stage; @@ -280,7 +282,7 @@ protected void deleteAction() { }); } - public void update(final Path newLauncherDirectory, final Path newDownloadDirectory, final BaseLauncherSettings newLauncherSettings, + public void update(final Path newLauncherDirectory, final Path newDownloadDirectory, final LauncherSettings newLauncherSettings, final PackageManager newPackageManager, final Stage newStage, final HostServices hostServices) { this.launcherDirectory = newLauncherDirectory; this.launcherSettings = newLauncherSettings; @@ -452,8 +454,9 @@ private void initButtons() { */ private void close() { logger.debug("Dispose launcher frame..."); + final Path settingsFile = launcherDirectory.resolve(Settings.DEFAULT_FILE_NAME); try { - launcherSettings.store(); + Settings.store(launcherSettings, settingsFile); } catch (IOException e) { logger.warn("Could not store current launcher settings!"); } diff --git a/src/main/java/org/terasology/launcher/gui/javafx/SettingsController.java b/src/main/java/org/terasology/launcher/gui/javafx/SettingsController.java index 4f123a878..2d404d03a 100644 --- a/src/main/java/org/terasology/launcher/gui/javafx/SettingsController.java +++ b/src/main/java/org/terasology/launcher/gui/javafx/SettingsController.java @@ -33,6 +33,8 @@ import org.slf4j.event.Level; import org.terasology.launcher.packages.PackageManager; import org.terasology.launcher.settings.BaseLauncherSettings; +import org.terasology.launcher.settings.LauncherSettings; +import org.terasology.launcher.settings.Settings; import org.terasology.launcher.util.BundleUtils; import org.terasology.launcher.util.JavaHeapSize; import org.terasology.launcher.util.Languages; @@ -49,7 +51,7 @@ public class SettingsController { private static final Logger logger = LoggerFactory.getLogger(SettingsController.class); private Path launcherDirectory; - private BaseLauncherSettings launcherSettings; + private LauncherSettings launcherSettings; private PackageManager packageManager; private ApplicationController appController; @@ -163,10 +165,12 @@ protected void saveSettingsAction(ActionEvent event) { } // store changed settings + final Path settingsFile = launcherDirectory.resolve(Settings.DEFAULT_FILE_NAME); try { - launcherSettings.store(); + Settings.store(launcherSettings, settingsFile); } catch (IOException e) { - logger.error("The launcher settings can not be stored! '{}'", launcherSettings.getLauncherSettingsFilePath(), e); + //TODO: unify error handling, probably to Settings a.k.a. SettingsController? + logger.error("The launcher settings cannot be stored! '{}'", settingsFile, e); Dialogs.showError(stage, BundleUtils.getLabel("message_error_storeSettings")); } finally { ((Node) event.getSource()).getScene().getWindow().hide(); @@ -206,7 +210,7 @@ protected void updateInitialHeapSizeBox() { } } - void initialize(final Path newLauncherDirectory, final BaseLauncherSettings newLauncherSettings, + void initialize(final Path newLauncherDirectory, final LauncherSettings newLauncherSettings, final PackageManager newPackageManager, final Stage newStage, final ApplicationController newAppController) { this.launcherDirectory = newLauncherDirectory; this.launcherSettings = newLauncherSettings; diff --git a/src/main/java/org/terasology/launcher/settings/BaseLauncherSettings.java b/src/main/java/org/terasology/launcher/settings/BaseLauncherSettings.java index f5e6b99e2..6bbc19ce9 100644 --- a/src/main/java/org/terasology/launcher/settings/BaseLauncherSettings.java +++ b/src/main/java/org/terasology/launcher/settings/BaseLauncherSettings.java @@ -22,12 +22,8 @@ import org.terasology.launcher.util.JavaHeapSize; import org.terasology.launcher.util.Languages; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Locale; @@ -39,7 +35,7 @@ * @deprecated to be replaced by {@link org.terasology.launcher.config.Config} */ @Deprecated -public final class BaseLauncherSettings extends AbstractLauncherSettings { +public final class BaseLauncherSettings extends LauncherSettings { public static final String USER_JAVA_PARAMETERS_DEFAULT = "-XX:MaxGCPauseMillis=20"; public static final String USER_GAME_PARAMETERS_DEFAULT = ""; @@ -76,49 +72,19 @@ public final class BaseLauncherSettings extends AbstractLauncherSettings { private static final Logger logger = LoggerFactory.getLogger(BaseLauncherSettings.class); - private static final String COMMENT_SETTINGS = "Terasology Launcher - Settings"; private static final String WARN_MSG_INVALID_VALUE = "Invalid value '{}' for the parameter '{}'!"; private static final Level LOG_LEVEL_DEFAULT = Level.INFO; - private final Path launcherSettingsFile; private final Properties properties; - public BaseLauncherSettings(Path launcherDirectory) { - launcherSettingsFile = launcherDirectory.resolve(LAUNCHER_SETTINGS_FILE_NAME); - properties = new Properties(); - } - - public String getLauncherSettingsFilePath() { - return launcherSettingsFile.toString(); - } - - @Override - public synchronized void load() throws IOException { - if (Files.exists(launcherSettingsFile)) { - logger.debug("Load the launcher settings from the file '{}'.", launcherSettingsFile); - - // load settings - try (InputStream inputStream = Files.newInputStream(launcherSettingsFile)) { - properties.load(inputStream); - } - } + BaseLauncherSettings(Properties properties) { + this.properties = properties; } @Override - public synchronized void store() throws IOException { - logger.trace("Store the launcher settings into the file '{}'.", launcherSettingsFile); - - // create directory - if (Files.notExists(launcherSettingsFile.getParent())) { - Files.createDirectories(launcherSettingsFile.getParent()); - } - - // store settings - try (OutputStream outputStream = Files.newOutputStream(launcherSettingsFile)) { - properties.store(outputStream, COMMENT_SETTINGS); - logger.trace("Stored settings: {}", this); - } + public Properties getProperties() { + return properties; } // --------------------------------------------------------------------- // diff --git a/src/main/java/org/terasology/launcher/settings/AbstractLauncherSettings.java b/src/main/java/org/terasology/launcher/settings/LauncherSettings.java similarity index 96% rename from src/main/java/org/terasology/launcher/settings/AbstractLauncherSettings.java rename to src/main/java/org/terasology/launcher/settings/LauncherSettings.java index f16c234e6..3f4446461 100644 --- a/src/main/java/org/terasology/launcher/settings/AbstractLauncherSettings.java +++ b/src/main/java/org/terasology/launcher/settings/LauncherSettings.java @@ -26,18 +26,17 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; +import java.util.Properties; /** * @deprecated to be replaced by {@link org.terasology.launcher.config.Config} */ @Deprecated -public abstract class AbstractLauncherSettings { +public abstract class LauncherSettings { - private static final Logger logger = LoggerFactory.getLogger(AbstractLauncherSettings.class); + private static final Logger logger = LoggerFactory.getLogger(LauncherSettings.class); - public abstract void load() throws IOException; - - public abstract void store() throws IOException; + public abstract Properties getProperties(); public synchronized void init() { logger.trace("Init launcher settings ..."); diff --git a/src/main/java/org/terasology/launcher/settings/LauncherSettingsValidator.java b/src/main/java/org/terasology/launcher/settings/LauncherSettingsValidator.java index da9dbac28..8d84459cf 100644 --- a/src/main/java/org/terasology/launcher/settings/LauncherSettingsValidator.java +++ b/src/main/java/org/terasology/launcher/settings/LauncherSettingsValidator.java @@ -67,12 +67,12 @@ private static String removeUnsupportedJvmParameters(final List params) } /** - * Validates an {@link AbstractLauncherSettings} instance against a list of rules. + * Validates an {@link LauncherSettings} instance against a list of rules. * Also applies a correction to the settings if it breaks any rule. * * @param settings the settings to be validated */ - public static void validate(AbstractLauncherSettings settings) { + public static void validate(LauncherSettings settings) { for (SettingsValidationRule rule : RULES) { if (rule.isBrokenBy(settings)) { logger.warn(rule.getInvalidationMessage()); diff --git a/src/main/java/org/terasology/launcher/settings/Settings.java b/src/main/java/org/terasology/launcher/settings/Settings.java new file mode 100644 index 000000000..bc6a2e5cf --- /dev/null +++ b/src/main/java/org/terasology/launcher/settings/Settings.java @@ -0,0 +1,54 @@ +package org.terasology.launcher.settings; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Properties; + +//TODO: should this be called `SettingsController` and also carry out some UI handling, e.g., displaying error messages +// to the user? +public class Settings { + public static final String DEFAULT_FILE_NAME = "TerasologyLauncherSettings.properties"; + + private static final Logger logger = LoggerFactory.getLogger(Settings.class); + + private static final String COMMENT_SETTINGS = "Terasology Launcher - Settings"; + + private Settings() { + } + + public static LauncherSettings load(final Path path) { + if (Files.exists(path)) { + logger.debug("Loading launcher settings from '{}'.", path); + + // load settings + try (InputStream inputStream = Files.newInputStream(path)) { + Properties properties = new Properties(); + properties.load(inputStream); + return new BaseLauncherSettings(properties); + } catch (IOException e) { + logger.error("Error while loading launcher settings from file.", e); + } + } + return null; + } + + public static synchronized void store(final LauncherSettings settings, final Path path) throws IOException { + logger.debug("Writing launcher settings to '{}'.", path); + if (Files.notExists(path.getParent())) { + Files.createDirectories(path.getParent()); + } + try (OutputStream outputStream = Files.newOutputStream(path)) { + settings.getProperties().store(outputStream, COMMENT_SETTINGS); + } + } + + public static LauncherSettings getDefault() { + return new BaseLauncherSettings(new Properties()); + }; +} diff --git a/src/main/java/org/terasology/launcher/settings/SettingsValidationRule.java b/src/main/java/org/terasology/launcher/settings/SettingsValidationRule.java index c9fc4ea2e..4610bfef0 100644 --- a/src/main/java/org/terasology/launcher/settings/SettingsValidationRule.java +++ b/src/main/java/org/terasology/launcher/settings/SettingsValidationRule.java @@ -23,21 +23,21 @@ * Provides methods to check settings values and correct the invalid ones. */ public class SettingsValidationRule { - private final Predicate condition; + private final Predicate condition; private final String invalidationMessage; - private final Consumer correction; + private final Consumer correction; public SettingsValidationRule( - Predicate condition, + Predicate condition, String invalidationMessage, - Consumer correction + Consumer correction ) { this.condition = condition; this.invalidationMessage = invalidationMessage; this.correction = correction; } - public boolean isBrokenBy(AbstractLauncherSettings settings) { + public boolean isBrokenBy(LauncherSettings settings) { return !condition.test(settings); } @@ -45,7 +45,7 @@ public String getInvalidationMessage() { return invalidationMessage; } - public void correct(AbstractLauncherSettings settings) { + public void correct(LauncherSettings settings) { correction.accept(settings); } } diff --git a/src/test/java/org/terasology/launcher/settings/TestBaseLauncherSettings.java b/src/test/java/org/terasology/launcher/settings/TestLauncherSettings.java similarity index 96% rename from src/test/java/org/terasology/launcher/settings/TestBaseLauncherSettings.java rename to src/test/java/org/terasology/launcher/settings/TestLauncherSettings.java index a30d2f1b1..4081c6f8e 100644 --- a/src/test/java/org/terasology/launcher/settings/TestBaseLauncherSettings.java +++ b/src/test/java/org/terasology/launcher/settings/TestLauncherSettings.java @@ -43,7 +43,7 @@ import static org.terasology.launcher.settings.BaseLauncherSettings.PROPERTY_USER_GAME_PARAMETERS; import static org.terasology.launcher.settings.BaseLauncherSettings.PROPERTY_USER_JAVA_PARAMETERS; -public class TestBaseLauncherSettings { +public class TestLauncherSettings { @TempDir Path tempDirectory; @TempDir @@ -51,7 +51,7 @@ public class TestBaseLauncherSettings { @TempDir Path gameDataDirectory; - private BaseLauncherSettings baseLauncherSettings; + private LauncherSettings baseLauncherSettings; private Path testPropertiesFile; private String locale; @@ -82,7 +82,7 @@ private void assertPropertiesEqual() throws Exception { public void setup() throws Exception { testPropertiesFile = tempDirectory.resolve(BaseLauncherSettings.LAUNCHER_SETTINGS_FILE_NAME); - baseLauncherSettings = new BaseLauncherSettings(tempDirectory); + baseLauncherSettings = Settings.getDefault(); } @Test @@ -117,7 +117,7 @@ public void testInitWithValues() throws Exception { testProperties.store(output, null); } - baseLauncherSettings.load(); + baseLauncherSettings = Settings.load(testPropertiesFile); baseLauncherSettings.init(); assertPropertiesEqual(); } @@ -126,7 +126,7 @@ public void testInitWithValues() throws Exception { public void testInitDefault() throws Exception { //null properties file - baseLauncherSettings.load(); + baseLauncherSettings = Settings.getDefault(); baseLauncherSettings.init(); assertEquals(baseLauncherSettings.getLocale(), Languages.DEFAULT_LOCALE);