From da6178d49c687e3e5a11207e7024076ab23767eb Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Mon, 4 Nov 2024 22:25:49 +0000 Subject: [PATCH 1/3] migration: Pass migrations to Migrator in constructor The migrations are hard-coded in the Migrator making it impossible to add test where some migrations fail. --- .../java/bisq/evolution/migration/MigrationService.java | 6 +++++- .../src/main/java/bisq/evolution/migration/Migrator.java | 5 ++--- .../test/java/bisq/evolution/migration/MigratorTest.java | 3 ++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/evolution/src/main/java/bisq/evolution/migration/MigrationService.java b/evolution/src/main/java/bisq/evolution/migration/MigrationService.java index d5d2e691a8..13deab1f85 100644 --- a/evolution/src/main/java/bisq/evolution/migration/MigrationService.java +++ b/evolution/src/main/java/bisq/evolution/migration/MigrationService.java @@ -3,11 +3,14 @@ import bisq.common.application.ApplicationVersion; import bisq.common.application.Service; import bisq.common.platform.Version; +import bisq.evolution.migration.migrations.Migration; +import bisq.evolution.migration.migrations.MigrationsForV2_1_2; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.concurrent.CompletableFuture; public class MigrationService implements Service { @@ -26,7 +29,8 @@ public CompletableFuture initialize() { Version appVersion = ApplicationVersion.getVersion(); if (dataDirVersion.below(appVersion)) { - Migrator migrator = new Migrator(appVersion, dataDir); + List allMigrations = List.of(new MigrationsForV2_1_2()); + Migrator migrator = new Migrator(appVersion, dataDir, allMigrations); migrator.migrate(); } diff --git a/evolution/src/main/java/bisq/evolution/migration/Migrator.java b/evolution/src/main/java/bisq/evolution/migration/Migrator.java index 8c1513e5ac..928963d962 100644 --- a/evolution/src/main/java/bisq/evolution/migration/Migrator.java +++ b/evolution/src/main/java/bisq/evolution/migration/Migrator.java @@ -4,7 +4,6 @@ import bisq.common.platform.Version; import bisq.evolution.migration.migrations.Migration; import bisq.evolution.migration.migrations.MigrationFailedException; -import bisq.evolution.migration.migrations.MigrationsForV2_1_2; import lombok.extern.slf4j.Slf4j; import java.io.IOException; @@ -18,10 +17,10 @@ public class Migrator { private final Path dataDir; private final List allMigrations; - public Migrator(Version appVersion, Path dataDir) { + public Migrator(Version appVersion, Path dataDir, List allMigrations) { this.appVersion = appVersion; this.dataDir = dataDir; - this.allMigrations = List.of(new MigrationsForV2_1_2()); + this.allMigrations = allMigrations; } public void migrate() { diff --git a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java b/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java index 92173163c7..5e7c90b24d 100644 --- a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java +++ b/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; import static org.assertj.core.api.Assertions.assertThat; @@ -19,7 +20,7 @@ void migrationSuccess(@TempDir Path dataDir) throws IOException { Files.writeString(versionFilePath, dataDirVersion.toString()); Version appVersion = ApplicationVersion.getVersion(); - Migrator migrator = new Migrator(appVersion, dataDir); + Migrator migrator = new Migrator(appVersion, dataDir, Collections.emptyList()); migrator.migrate(); From b397b7675345c0070b6c9aa5ef059452e06b42b2 Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Mon, 4 Nov 2024 22:32:48 +0000 Subject: [PATCH 2/3] migration: Test migration failures --- .../migrations/MigrationFailedException.java | 4 +++ .../evolution/migration/MigratorTest.java | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java b/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java index 1a7f8c6cd8..c01effde11 100644 --- a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java +++ b/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java @@ -1,6 +1,10 @@ package bisq.evolution.migration.migrations; public class MigrationFailedException extends RuntimeException { + public MigrationFailedException(String message) { + super(message); + } + public MigrationFailedException(Throwable cause) { super(cause); } diff --git a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java b/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java index 5e7c90b24d..6f2a9594fa 100644 --- a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java +++ b/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java @@ -2,6 +2,8 @@ import bisq.common.application.ApplicationVersion; import bisq.common.platform.Version; +import bisq.evolution.migration.migrations.Migration; +import bisq.evolution.migration.migrations.MigrationFailedException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -9,6 +11,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -27,4 +30,30 @@ void migrationSuccess(@TempDir Path dataDir) throws IOException { String readVersion = Files.readString(dataDir.resolve("version")); assertThat(readVersion).isEqualTo(appVersion.toString()); } + + @Test + void migrationFailure(@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(); + var migration = new Migration() { + @Override + public void run(Path dataDir) { + throw new MigrationFailedException("Migration failed."); + } + + @Override + public Version getVersion() { + return new Version("2.1.1"); + } + }; + + Migrator migrator = new Migrator(appVersion, dataDir, List.of(migration, migration)); + migrator.migrate(); + + String readVersion = Files.readString(dataDir.resolve("version")); + assertThat(readVersion).isEqualTo(dataDirVersion.toString()); + } } From e1b4e49602a5090b3712436d010a9cea5849bfbe Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Mon, 4 Nov 2024 22:36:00 +0000 Subject: [PATCH 3/3] migration: Don't keep MigrationService in memory The MigrationService isn't needed anymore after the data directory has been migrated. This change removes the ApplicationService's reference to the MigrationService after it has run. --- .../main/java/bisq/application/ApplicationService.java | 9 ++++++++- .../bisq/application}/migration/MigrationService.java | 6 +++--- .../main/java/bisq/application}/migration/Migrator.java | 6 +++--- .../application}/migration/migrations/Migration.java | 2 +- .../migration/migrations/MigrationFailedException.java | 2 +- .../migration/migrations/MigrationsForV2_1_2.java | 2 +- .../application}/migration/MigrationServiceTests.java | 3 ++- .../java/bisq/application}/migration/MigratorTest.java | 6 +++--- .../migration/migrations/MigrationsForV2_1_2Tests.java | 3 ++- .../java/bisq/desktop_app/DesktopApplicationService.java | 4 +--- .../bisq/oracle_node/OracleNodeApplicationService.java | 4 +--- .../java/bisq/rest_api/RestApiApplicationService.java | 4 +--- .../java/bisq/seed_node/SeedNodeApplicationService.java | 4 +--- .../java_se/application/JavaSeApplicationService.java | 3 --- 14 files changed, 28 insertions(+), 30 deletions(-) rename {evolution/src/main/java/bisq/evolution => application/src/main/java/bisq/application}/migration/MigrationService.java (91%) rename {evolution/src/main/java/bisq/evolution => application/src/main/java/bisq/application}/migration/Migrator.java (90%) rename {evolution/src/main/java/bisq/evolution => application/src/main/java/bisq/application}/migration/migrations/Migration.java (76%) rename {evolution/src/main/java/bisq/evolution => application/src/main/java/bisq/application}/migration/migrations/MigrationFailedException.java (83%) rename {evolution/src/main/java/bisq/evolution => application/src/main/java/bisq/application}/migration/migrations/MigrationsForV2_1_2.java (93%) rename {evolution/src/test/java/bisq/evolution => application/src/test/java/bisq/application}/migration/MigrationServiceTests.java (94%) rename {evolution/src/test/java/bisq/evolution => application/src/test/java/bisq/application}/migration/MigratorTest.java (92%) rename {evolution/src/test/java/bisq/evolution => application/src/test/java/bisq/application}/migration/migrations/MigrationsForV2_1_2Tests.java (97%) diff --git a/application/src/main/java/bisq/application/ApplicationService.java b/application/src/main/java/bisq/application/ApplicationService.java index 561915cb76..09e66bac25 100644 --- a/application/src/main/java/bisq/application/ApplicationService.java +++ b/application/src/main/java/bisq/application/ApplicationService.java @@ -29,6 +29,7 @@ import bisq.common.logging.AsciiLogo; import bisq.common.logging.LogSetup; import bisq.common.util.ExceptionUtil; +import bisq.application.migration.MigrationService; import bisq.i18n.Res; import bisq.persistence.PersistenceService; import com.typesafe.config.ConfigFactory; @@ -122,6 +123,7 @@ public Config(Path baseDir, protected final Config config; @Getter protected final PersistenceService persistenceService; + private Optional migrationService; private FileLock instanceLock; public ApplicationService(String configFileName, String[] args, Path userDataDir) { @@ -178,6 +180,7 @@ public ApplicationService(String configFileName, String[] args, Path userDataDir String absoluteDataDirPath = dataDir.toAbsolutePath().toString(); persistenceService = new PersistenceService(absoluteDataDirPath); + migrationService = Optional.of(new MigrationService(dataDir)); } private void checkInstanceLock() { @@ -204,7 +207,11 @@ public CompletableFuture readAllPersisted() { return persistenceService.readAllPersisted(); } - public abstract CompletableFuture initialize(); + public CompletableFuture initialize() { + CompletableFuture completableFuture = migrationService.orElseThrow().initialize(); + migrationService = Optional.empty(); + return completableFuture; + } public abstract CompletableFuture shutdown(); diff --git a/evolution/src/main/java/bisq/evolution/migration/MigrationService.java b/application/src/main/java/bisq/application/migration/MigrationService.java similarity index 91% rename from evolution/src/main/java/bisq/evolution/migration/MigrationService.java rename to application/src/main/java/bisq/application/migration/MigrationService.java index 13deab1f85..2fdfd69aa9 100644 --- a/evolution/src/main/java/bisq/evolution/migration/MigrationService.java +++ b/application/src/main/java/bisq/application/migration/MigrationService.java @@ -1,10 +1,10 @@ -package bisq.evolution.migration; +package bisq.application.migration; +import bisq.application.migration.migrations.Migration; +import bisq.application.migration.migrations.MigrationsForV2_1_2; import bisq.common.application.ApplicationVersion; import bisq.common.application.Service; import bisq.common.platform.Version; -import bisq.evolution.migration.migrations.Migration; -import bisq.evolution.migration.migrations.MigrationsForV2_1_2; import java.io.File; import java.io.IOException; diff --git a/evolution/src/main/java/bisq/evolution/migration/Migrator.java b/application/src/main/java/bisq/application/migration/Migrator.java similarity index 90% rename from evolution/src/main/java/bisq/evolution/migration/Migrator.java rename to application/src/main/java/bisq/application/migration/Migrator.java index 928963d962..dd3a5cc0c2 100644 --- a/evolution/src/main/java/bisq/evolution/migration/Migrator.java +++ b/application/src/main/java/bisq/application/migration/Migrator.java @@ -1,9 +1,9 @@ -package bisq.evolution.migration; +package bisq.application.migration; import bisq.common.application.ApplicationVersion; import bisq.common.platform.Version; -import bisq.evolution.migration.migrations.Migration; -import bisq.evolution.migration.migrations.MigrationFailedException; +import bisq.application.migration.migrations.Migration; +import bisq.application.migration.migrations.MigrationFailedException; import lombok.extern.slf4j.Slf4j; import java.io.IOException; diff --git a/evolution/src/main/java/bisq/evolution/migration/migrations/Migration.java b/application/src/main/java/bisq/application/migration/migrations/Migration.java similarity index 76% rename from evolution/src/main/java/bisq/evolution/migration/migrations/Migration.java rename to application/src/main/java/bisq/application/migration/migrations/Migration.java index 0f5261421f..bfdf3bbb45 100644 --- a/evolution/src/main/java/bisq/evolution/migration/migrations/Migration.java +++ b/application/src/main/java/bisq/application/migration/migrations/Migration.java @@ -1,4 +1,4 @@ -package bisq.evolution.migration.migrations; +package bisq.application.migration.migrations; import bisq.common.platform.Version; diff --git a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java b/application/src/main/java/bisq/application/migration/migrations/MigrationFailedException.java similarity index 83% rename from evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java rename to application/src/main/java/bisq/application/migration/migrations/MigrationFailedException.java index c01effde11..cc7ea26d40 100644 --- a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationFailedException.java +++ b/application/src/main/java/bisq/application/migration/migrations/MigrationFailedException.java @@ -1,4 +1,4 @@ -package bisq.evolution.migration.migrations; +package bisq.application.migration.migrations; public class MigrationFailedException extends RuntimeException { public MigrationFailedException(String message) { diff --git a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2.java b/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java similarity index 93% rename from evolution/src/main/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2.java rename to application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java index f5f09cc485..76a94db2a4 100644 --- a/evolution/src/main/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2.java +++ b/application/src/main/java/bisq/application/migration/migrations/MigrationsForV2_1_2.java @@ -1,4 +1,4 @@ -package bisq.evolution.migration.migrations; +package bisq.application.migration.migrations; import bisq.common.file.FileUtils; import bisq.common.platform.OS; diff --git a/evolution/src/test/java/bisq/evolution/migration/MigrationServiceTests.java b/application/src/test/java/bisq/application/migration/MigrationServiceTests.java similarity index 94% rename from evolution/src/test/java/bisq/evolution/migration/MigrationServiceTests.java rename to application/src/test/java/bisq/application/migration/MigrationServiceTests.java index 976b9b1560..053b35ad57 100644 --- a/evolution/src/test/java/bisq/evolution/migration/MigrationServiceTests.java +++ b/application/src/test/java/bisq/application/migration/MigrationServiceTests.java @@ -1,5 +1,6 @@ -package bisq.evolution.migration; +package bisq.application.migration; +import bisq.application.migration.MigrationService; import bisq.common.platform.InvalidVersionException; import bisq.common.platform.Version; import org.junit.jupiter.api.Test; diff --git a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java b/application/src/test/java/bisq/application/migration/MigratorTest.java similarity index 92% rename from evolution/src/test/java/bisq/evolution/migration/MigratorTest.java rename to application/src/test/java/bisq/application/migration/MigratorTest.java index 6f2a9594fa..4258c2fb73 100644 --- a/evolution/src/test/java/bisq/evolution/migration/MigratorTest.java +++ b/application/src/test/java/bisq/application/migration/MigratorTest.java @@ -1,9 +1,9 @@ -package bisq.evolution.migration; +package bisq.application.migration; +import bisq.application.migration.migrations.Migration; +import bisq.application.migration.migrations.MigrationFailedException; import bisq.common.application.ApplicationVersion; import bisq.common.platform.Version; -import bisq.evolution.migration.migrations.Migration; -import bisq.evolution.migration.migrations.MigrationFailedException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; diff --git a/evolution/src/test/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2Tests.java b/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java similarity index 97% rename from evolution/src/test/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2Tests.java rename to application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java index d4de37bc9b..db67326268 100644 --- a/evolution/src/test/java/bisq/evolution/migration/migrations/MigrationsForV2_1_2Tests.java +++ b/application/src/test/java/bisq/application/migration/migrations/MigrationsForV2_1_2Tests.java @@ -1,5 +1,6 @@ -package bisq.evolution.migration.migrations; +package bisq.application.migration.migrations; +import bisq.application.migration.migrations.MigrationsForV2_1_2; import bisq.common.platform.Version; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; diff --git a/apps/desktop/desktop-app/src/main/java/bisq/desktop_app/DesktopApplicationService.java b/apps/desktop/desktop-app/src/main/java/bisq/desktop_app/DesktopApplicationService.java index 4a9b42977d..7a381865f7 100644 --- a/apps/desktop/desktop-app/src/main/java/bisq/desktop_app/DesktopApplicationService.java +++ b/apps/desktop/desktop-app/src/main/java/bisq/desktop_app/DesktopApplicationService.java @@ -244,8 +244,7 @@ private Optional findSystemNotificationDelegate() @Override public CompletableFuture initialize() { - return migrationService.initialize() - .thenCompose(result -> memoryReportService.initialize()) + return memoryReportService.initialize() .thenCompose(result -> securityService.initialize()) .thenCompose(result -> { setState(State.INITIALIZE_NETWORK); @@ -338,7 +337,6 @@ public CompletableFuture shutdown() { .orElse(CompletableFuture.completedFuture(true))) .thenCompose(result -> securityService.shutdown().exceptionally(this::logError)) .thenCompose(result -> memoryReportService.shutdown().exceptionally(this::logError)) - .thenCompose(result -> migrationService.shutdown().exceptionally(this::logError)) .orTimeout(SHUTDOWN_TIMEOUT_SEC, TimeUnit.SECONDS) .handle((result, throwable) -> { if (throwable == null) { diff --git a/apps/oracle-node-app/src/main/java/bisq/oracle_node/OracleNodeApplicationService.java b/apps/oracle-node-app/src/main/java/bisq/oracle_node/OracleNodeApplicationService.java index 2ac2b2ef39..09a9c6ae81 100644 --- a/apps/oracle-node-app/src/main/java/bisq/oracle_node/OracleNodeApplicationService.java +++ b/apps/oracle-node-app/src/main/java/bisq/oracle_node/OracleNodeApplicationService.java @@ -82,8 +82,7 @@ public OracleNodeApplicationService(String[] args) { @Override public CompletableFuture initialize() { - return migrationService.initialize() - .thenCompose(result -> memoryReportService.initialize()) + return memoryReportService.initialize() .thenCompose(result -> securityService.initialize()) .thenCompose(result -> networkService.initialize()) .thenCompose(result -> identityService.initialize()) @@ -110,7 +109,6 @@ public CompletableFuture shutdown() { .thenCompose(result -> networkService.shutdown()) .thenCompose(result -> securityService.shutdown()) .thenCompose(result -> memoryReportService.shutdown()) - .thenCompose(result -> migrationService.shutdown()) .orTimeout(2, TimeUnit.MINUTES) .handle((result, throwable) -> throwable == null) .join()); diff --git a/apps/rest-api-app/src/main/java/bisq/rest_api/RestApiApplicationService.java b/apps/rest-api-app/src/main/java/bisq/rest_api/RestApiApplicationService.java index cf42857939..9d174b2a32 100644 --- a/apps/rest-api-app/src/main/java/bisq/rest_api/RestApiApplicationService.java +++ b/apps/rest-api-app/src/main/java/bisq/rest_api/RestApiApplicationService.java @@ -173,8 +173,7 @@ public RestApiApplicationService(String[] args) { @Override public CompletableFuture initialize() { - return migrationService.initialize() - .thenCompose(result -> memoryReportService.initialize()) + return memoryReportService.initialize() .thenCompose(result -> securityService.initialize()) .thenCompose(result -> { setState(State.INITIALIZE_NETWORK); @@ -255,7 +254,6 @@ public CompletableFuture shutdown() { .orElse(CompletableFuture.completedFuture(true))) .thenCompose(result -> securityService.shutdown()) .thenCompose(result -> memoryReportService.shutdown()) - .thenCompose(result -> migrationService.shutdown()) .orTimeout(10, TimeUnit.SECONDS) .handle((result, throwable) -> throwable == null) .join()); diff --git a/apps/seed-node-app/src/main/java/bisq/seed_node/SeedNodeApplicationService.java b/apps/seed-node-app/src/main/java/bisq/seed_node/SeedNodeApplicationService.java index c47ba4eb46..5f18ee4932 100644 --- a/apps/seed-node-app/src/main/java/bisq/seed_node/SeedNodeApplicationService.java +++ b/apps/seed-node-app/src/main/java/bisq/seed_node/SeedNodeApplicationService.java @@ -76,8 +76,7 @@ public SeedNodeApplicationService(String[] args) { @Override public CompletableFuture initialize() { - return migrationService.initialize() - .thenCompose(result -> memoryReportService.initialize()) + return memoryReportService.initialize() .thenCompose(result -> securityService.initialize()) .thenCompose(result -> networkService.initialize()) .thenCompose(result -> identityService.initialize()) @@ -105,7 +104,6 @@ public CompletableFuture shutdown() { .thenCompose(result -> networkService.shutdown()) .thenCompose(result -> securityService.shutdown()) .thenCompose(result -> memoryReportService.shutdown()) - .thenCompose(result -> migrationService.shutdown()) .orTimeout(10, TimeUnit.SECONDS) .handle((result, throwable) -> { if (throwable != null) { diff --git a/java-se/src/main/java/bisq/java_se/application/JavaSeApplicationService.java b/java-se/src/main/java/bisq/java_se/application/JavaSeApplicationService.java index 377799dd30..be2bd5f7cd 100644 --- a/java-se/src/main/java/bisq/java_se/application/JavaSeApplicationService.java +++ b/java-se/src/main/java/bisq/java_se/application/JavaSeApplicationService.java @@ -21,7 +21,6 @@ import bisq.common.facades.FacadeProvider; import bisq.common.platform.MemoryReportService; import bisq.common.platform.PlatformUtils; -import bisq.evolution.migration.MigrationService; import bisq.java_se.facades.JavaSeGuavaFacade; import bisq.java_se.facades.JavaSeJdkFacade; import bisq.java_se.jvm.JvmMemoryReportService; @@ -31,7 +30,6 @@ @Getter @Slf4j public abstract class JavaSeApplicationService extends ApplicationService { - protected final MigrationService migrationService; protected final MemoryReportService memoryReportService; public JavaSeApplicationService(String configFileName, String[] args) { @@ -43,7 +41,6 @@ public JavaSeApplicationService(String configFileName, String[] args) { FacadeProvider.setGuavaFacade(new JavaSeGuavaFacade()); FacadeProvider.setJdkFacade(new JavaSeJdkFacade()); - migrationService = new MigrationService(getConfig().getBaseDir()); memoryReportService = new JvmMemoryReportService(getConfig().getMemoryReportIntervalSec(), getConfig().isIncludeThreadListInMemoryReport()); } }