From 59cd878608fca66125bd9414aecfc85834e8f7e4 Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Tue, 22 Oct 2024 22:08:18 +0000 Subject: [PATCH] migration: Write new data directory version if all migrations succeed Migrations are applied if the app version number is higher than the data directory version number. Currently, the only existing migration writes the new version number to the data directory after applying its migration. Moving the data directory version write to the Migrator makes the code future-proof when we have multiple migrations. --- .../bisq/application/migration/Migrator.java | 23 ++++++++++++++- .../migrations/MigrationsForV2_1_2.java | 5 ---- .../application/migration/MigratorTest.java | 29 +++++++++++++++++++ .../migrations/MigrationsForV2_1_2Tests.java | 18 ++---------- 4 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 application/src/test/java/bisq/application/migration/MigratorTest.java diff --git a/application/src/main/java/bisq/application/migration/Migrator.java b/application/src/main/java/bisq/application/migration/Migrator.java index fec64da7d9..b945d941c9 100644 --- a/application/src/main/java/bisq/application/migration/Migrator.java +++ b/application/src/main/java/bisq/application/migration/Migrator.java @@ -1,10 +1,14 @@ package bisq.application.migration; import bisq.application.migration.migrations.Migration; +import bisq.application.migration.migrations.MigrationFailedException; import bisq.application.migration.migrations.MigrationsForV2_1_2; +import bisq.common.application.ApplicationVersion; import bisq.common.platform.Version; import lombok.extern.slf4j.Slf4j; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -21,11 +25,28 @@ public Migrator(Version appVersion, Path dataDir) { } public void migrate() { + boolean allMigrationsSucceeded = true; + for (Migration migration : allMigrations) { Version migrationVersion = migration.getVersion(); if (migrationVersion.belowOrEqual(appVersion)) { log.info("Running {} migrations.", migrationVersion); - migration.run(dataDir); + + try { + migration.run(dataDir); + } catch (MigrationFailedException e) { + log.error("Migration failed.", e); + allMigrationsSucceeded = false; + } + } + } + + if (allMigrationsSucceeded) { + try { + Path versionFilePath = dataDir.resolve("version"); + Files.writeString(versionFilePath, ApplicationVersion.getVersion().toString()); + } catch (IOException e) { + throw new MigrationFailedException(e); } } } diff --git a/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java b/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java index 2638b55049..76a94db2a4 100644 --- a/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java +++ b/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java @@ -1,12 +1,10 @@ package bisq.application.migration.migrations; -import bisq.common.application.ApplicationVersion; import bisq.common.file.FileUtils; import bisq.common.platform.OS; import bisq.common.platform.Version; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; public class MigrationsForV2_1_2 implements Migration { @@ -18,9 +16,6 @@ public void run(Path dataDir) { Path torDataDir = dataDir.resolve("tor"); FileUtils.deleteFileOrDirectory(torDataDir); } - - Path versionFilePath = dataDir.resolve("version"); - Files.writeString(versionFilePath, ApplicationVersion.getVersion().toString()); } catch (IOException e) { throw new MigrationFailedException(e); } diff --git a/application/src/test/java/bisq/application/migration/MigratorTest.java b/application/src/test/java/bisq/application/migration/MigratorTest.java new file mode 100644 index 0000000000..cc7016ed95 --- /dev/null +++ b/application/src/test/java/bisq/application/migration/MigratorTest.java @@ -0,0 +1,29 @@ +package bisq.application.migration; + +import bisq.common.application.ApplicationVersion; +import bisq.common.platform.Version; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +public class MigratorTest { + @Test + void migrationSuccess(@TempDir Path dataDir) throws IOException { + Path versionFilePath = dataDir.resolve("version"); + Version dataDirVersion = new Version("2.1.0"); + Files.writeString(versionFilePath, dataDirVersion.toString()); + + Version appVersion = ApplicationVersion.getVersion(); + Migrator migrator = new Migrator(appVersion, dataDir); + + migrator.migrate(); + + String readVersion = Files.readString(dataDir.resolve("version")); + assertThat(readVersion).isEqualTo(appVersion.toString()); + } +} diff --git a/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java b/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java index 0ad63987a8..bb7668b617 100644 --- a/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java +++ b/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java @@ -1,6 +1,5 @@ package bisq.application.migration.migrations; -import bisq.common.application.ApplicationVersion; import bisq.common.platform.Version; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; @@ -18,20 +17,17 @@ public class MigrationsForV2_1_2Tests { private final MigrationsForV2_1_2 migrationsForV212 = new MigrationsForV2_1_2(); @Test - void migrationTest(@TempDir Path dataDir) throws IOException { + void migrateEmptyDataDir(@TempDir Path dataDir) { Version version = migrationsForV212.getVersion(); Version expectedVersion = new Version("2.1.2"); assertThat(version).isEqualTo(expectedVersion); migrationsForV212.run(dataDir); - String readVersion = Files.readString(dataDir.resolve("version")); - assertThat(readVersion) - .isEqualTo(ApplicationVersion.getVersion().toString()); } @Test @EnabledOnOs({OS.MAC, OS.WINDOWS}) - void torFilesRemovalTestOnMacAndWindows(@TempDir Path dataDir) throws IOException { + void torFilesRemovalTestOnMacAndWindows(@TempDir Path dataDir) { Path torDataDir = dataDir.resolve("tor"); createFakeTorDataDir(torDataDir); @@ -46,15 +42,11 @@ void torFilesRemovalTestOnMacAndWindows(@TempDir Path dataDir) throws IOExceptio fileExists = pluggableTransportDir.resolve("README.SNOWFLAKE.md").toFile().exists(); assertThat(fileExists).isTrue(); - - Path versionFilePath = dataDir.resolve("version"); - String version = Files.readString(versionFilePath); - assertThat(version).isEqualTo(ApplicationVersion.getVersion().toString()); } @Test @EnabledOnOs(OS.LINUX) - void torFilesRemovalTestOnLinux(@TempDir Path dataDir) throws IOException { + void torFilesRemovalTestOnLinux(@TempDir Path dataDir) { Path torDataDir = dataDir.resolve("tor"); createFakeTorDataDir(torDataDir); @@ -69,10 +61,6 @@ void torFilesRemovalTestOnLinux(@TempDir Path dataDir) throws IOException { fileExists = pluggableTransportDir.resolve("README.SNOWFLAKE.md").toFile().exists(); assertThat(fileExists).isFalse(); - - Path versionFilePath = dataDir.resolve("version"); - String version = Files.readString(versionFilePath); - assertThat(version).isEqualTo(ApplicationVersion.getVersion().toString()); } private void createFakeTorDataDir(Path torDataDir) {