From 193ed783ffe9b57ded2d67befa2b6d626ed0ceb4 Mon Sep 17 00:00:00 2001 From: Rsl1122 Date: Tue, 8 Jan 2019 13:58:48 +0200 Subject: [PATCH] More tests against config update issues --- .../system/settings/changes/ConfigChange.java | 6 +- .../settings/changes/ConfigUpdater.java | 16 ++++- .../settings/changes/ConfigChangeTest.java | 9 +++ .../settings/changes/ConfigUpdaterTest.java | 62 +++++++++++++++++-- 4 files changed, 82 insertions(+), 11 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigChange.java b/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigChange.java index da48282aa..3923d8fe0 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigChange.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigChange.java @@ -33,7 +33,7 @@ public interface ConfigChange { class Moved extends Removed { - private final String newPath; + final String newPath; public Moved(String oldPath, String newPath) { super(oldPath); @@ -55,7 +55,7 @@ public interface ConfigChange { class Copied extends Removed { - private final String newPath; + final String newPath; public Copied(String oldPath, String newPath) { super(oldPath); @@ -74,7 +74,7 @@ public interface ConfigChange { } class Removed implements ConfigChange { - final String oldPath; + String oldPath; public Removed(String oldPath) { this.oldPath = oldPath; diff --git a/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigUpdater.java b/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigUpdater.java index 00da8699f..a58a183a4 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigUpdater.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/system/settings/changes/ConfigUpdater.java @@ -20,6 +20,7 @@ import com.djrapitops.plan.system.settings.config.Config; import com.djrapitops.plugin.logging.L; import com.djrapitops.plugin.logging.console.PluginLogger; import com.djrapitops.plugin.logging.error.ErrorHandler; +import com.google.common.annotations.VisibleForTesting; import javax.inject.Inject; import javax.inject.Singleton; @@ -46,7 +47,14 @@ public class ConfigUpdater { } public void applyConfigUpdate(Config config) throws IOException { - ConfigChange[] configEnhancementsPatch = new ConfigChange[]{ + ConfigChange[] configEnhancementsPatch = configEnhancementPatch(); + applyChanges(config, configEnhancementsPatch); + config.save(); + } + + @VisibleForTesting + ConfigChange[] configEnhancementPatch() { + return new ConfigChange[]{ new ConfigChange.Moved("Plugin.Locale", "Plugin.Logging.Locale"), new ConfigChange.Moved("Plugin.WriteNewLocaleFileOnEnable", "Plugin.Logging.Create_new_locale_file_on_next_enable"), new ConfigChange.Moved("Plugin.Debug", "Plugin.Logging.Debug"), @@ -120,7 +128,10 @@ public class ConfigUpdater { new ConfigChange.Removed("Customization"), new ConfigChange.Removed("Theme") }; - for (ConfigChange change : configEnhancementsPatch) { + } + + private void applyChanges(Config config, ConfigChange[] changes) { + for (ConfigChange change : changes) { try { if (!change.hasBeenApplied(config)) { change.apply(config); @@ -130,6 +141,5 @@ public class ConfigUpdater { errorHandler.log(L.WARN, this.getClass(), new IllegalStateException("Failed to apply config update: '" + change.getAppliedMessage() + "'", e)); } } - config.save(); } } diff --git a/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigChangeTest.java b/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigChangeTest.java index dc79480c3..182150dac 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigChangeTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigChangeTest.java @@ -70,6 +70,15 @@ class ConfigChangeTest { assertTrue(change.hasBeenApplied(config), "Did not recognize it has been applied"); } + @Test + void moveChangeRecognizesItDoesNotNeedToBeApplied() { + config = prepareConfig("Test: 'value'"); + + ConfigChange change = new ConfigChange.Moved("NonExisting", "MovedTo"); + + assertTrue(change.hasBeenApplied(config), "Did not recognize it has been applied"); + } + @Test void stringSettingWithQuotesIsMovedAsString() { config = prepareConfig("Test: 'value'"); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigUpdaterTest.java b/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigUpdaterTest.java index 95a3ac118..1e1c01848 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigUpdaterTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/system/settings/changes/ConfigUpdaterTest.java @@ -36,8 +36,16 @@ import java.io.IOException; import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Collection; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Tests for ConfigUpdater. + * + * @author Rsl1122 + */ @RunWith(JUnitPlatform.class) @ExtendWith(TempDirectory.class) class ConfigUpdaterTest { @@ -58,11 +66,11 @@ class ConfigUpdaterTest { oldConfig = tempDir.resolve("config.yml").toFile(); File configResource = TestResources.getTestResourceFile("config/4.5.2-config.yml", ConfigUpdater.class); - Files.copy(configResource.toPath(), oldConfig.toPath()); + Files.copy(configResource.toPath(), oldConfig.toPath(), StandardCopyOption.REPLACE_EXISTING); oldBungeeConfig = tempDir.resolve("bungeeconfig.yml").toFile(); File bungeeConfigResource = TestResources.getTestResourceFile("config/4.5.2-bungeeconfig.yml", ConfigUpdater.class); - Files.copy(bungeeConfigResource.toPath(), oldBungeeConfig.toPath()); + Files.copy(bungeeConfigResource.toPath(), oldBungeeConfig.toPath(), StandardCopyOption.REPLACE_EXISTING); newConfig = tempDir.resolve("newconfig.yml"); TestResources.copyResourceIntoFile(newConfig.toFile(), "/config.yml"); @@ -76,7 +84,10 @@ class ConfigUpdaterTest { @Test void serverConfigIsPatchedCorrectly() throws IOException, IllegalAccessException { - PlanConfig planConfig = new PlanConfig(oldConfig, null); + Path config = tempDir.resolve("oldconfig.yml"); + Files.copy(oldConfig.toPath(), config, StandardCopyOption.REPLACE_EXISTING); + + PlanConfig planConfig = new PlanConfig(config.toFile(), null); UNDER_TEST.applyConfigUpdate(planConfig); @@ -88,8 +99,11 @@ class ConfigUpdaterTest { } @Test - void bungeeConfigIsPatchedCorrectly() throws IOException, IllegalAccessException { - PlanConfig planConfig = new PlanConfig(oldBungeeConfig, null); + void proxyConfigIsPatchedCorrectly() throws IOException, IllegalAccessException { + Path config = tempDir.resolve("oldconfig.yml"); + Files.copy(oldBungeeConfig.toPath(), config, StandardCopyOption.REPLACE_EXISTING); + + PlanConfig planConfig = new PlanConfig(config.toFile(), null); UNDER_TEST.applyConfigUpdate(planConfig); @@ -106,4 +120,42 @@ class ConfigUpdaterTest { } } + @Test + void serverMoveChangesDoNotLeaveNewEmptyValues() throws IOException { + Path config = tempDir.resolve("oldconfig.yml"); + Files.copy(oldConfig.toPath(), config, StandardCopyOption.REPLACE_EXISTING); + + PlanConfig planConfig = new PlanConfig(config.toFile(), null); + + ConfigChange[] changes = UNDER_TEST.configEnhancementPatch(); + assertMoveChangesAreAppliedProperly(planConfig, changes); + } + + @Test + void proxyMoveChangesDoNotLeaveNewEmptyValues() throws IOException { + Path config = tempDir.resolve("oldconfig.yml"); + Files.copy(oldBungeeConfig.toPath(), config, StandardCopyOption.REPLACE_EXISTING); + + PlanConfig planConfig = new PlanConfig(config.toFile(), null); + + ConfigChange[] changes = UNDER_TEST.configEnhancementPatch(); + assertMoveChangesAreAppliedProperly(planConfig, changes); + } + + private void assertMoveChangesAreAppliedProperly(PlanConfig planConfig, ConfigChange[] changes) { + for (ConfigChange change : changes) { + if (change.hasBeenApplied(planConfig)) { + continue; + } + + if (change instanceof ConfigChange.Moved) { + ConfigChange.Moved move = (ConfigChange.Moved) change; + String expected = planConfig.getString(move.oldPath); + + move.apply(planConfig); + + assertEquals(expected, planConfig.getString(move.newPath)); + } + } + } } \ No newline at end of file