From 4ba26ee7b761faf69819ee3ed76f622204784d72 Mon Sep 17 00:00:00 2001 From: rbygrave Date: Mon, 2 Aug 2021 20:49:30 +1200 Subject: [PATCH] #2283 - ddl-generator - Refactor tidy DefaultDbMigration etc --- .../dbmigration/DefaultDbMigration.java | 89 +++++-------------- .../dbmigration/LastMigration.java | 2 +- .../DbMigrationDropHistoryTest.java | 4 +- .../dbmigration/DbMigrationGenerateTest.java | 4 +- 4 files changed, 25 insertions(+), 74 deletions(-) diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/DefaultDbMigration.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/DefaultDbMigration.java index 0a7b0edee..7643eeec2 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/DefaultDbMigration.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/DefaultDbMigration.java @@ -2,7 +2,6 @@ package io.ebeaninternal.dbmigration; import io.ebean.DB; import io.ebean.Database; -import io.ebean.EbeanServer; import io.ebean.annotation.Platform; import io.ebean.config.DatabaseConfig; import io.ebean.config.DbConstraintNaming; @@ -79,20 +78,12 @@ import static io.ebeaninternal.api.PlatformMatch.matchPlatform; public class DefaultDbMigration implements DbMigration { protected static final Logger logger = LoggerFactory.getLogger("io.ebean.GenerateMigration"); - private static final String initialVersion = "1.0"; - private static final String GENERATED_COMMENT = "THIS IS A GENERATED FILE - DO NOT MODIFY"; - private boolean logToSystemOut = true; - - /** - * Set to true if DefaultDbMigration run with online EbeanServer instance. - */ protected final boolean online; - + private boolean logToSystemOut = true; protected SpiEbeanServer server; - protected String pathToResources = "src/main/resources"; protected String migrationPath = "dbmigration"; @@ -101,15 +92,10 @@ public class DefaultDbMigration implements DbMigration { protected String modelSuffix = ".model.xml"; protected DatabasePlatform databasePlatform; - private boolean vanillaPlatform; - protected List platforms = new ArrayList<>(); - protected DatabaseConfig databaseConfig; - protected DbConstraintNaming constraintNaming; - protected Boolean strictMode; protected Boolean includeGeneratedFileComment; protected String header; @@ -119,7 +105,6 @@ public class DefaultDbMigration implements DbMigration { protected String generatePendingDrop; private boolean addForeignKeySkipCheck; private int lockTimeoutSeconds; - protected boolean includeBuiltInPartitioning = true; /** @@ -129,19 +114,6 @@ public class DefaultDbMigration implements DbMigration { this.online = false; } - /** - * Create using online EbeanServer. - */ - public DefaultDbMigration(EbeanServer server) { - this.online = true; - setServer(server); - } - - /** - * Set the path from the current working directory to the application resources. - *

- * This defaults to maven style 'src/main/resources'. - */ @Override public void setPathToResources(String pathToResources) { this.pathToResources = pathToResources; @@ -152,19 +124,12 @@ public class DefaultDbMigration implements DbMigration { this.migrationPath = migrationPath; } - /** - * Set the server to use to determine the current model. - * Typically this is not called explicitly. - */ @Override public void setServer(Database database) { this.server = (SpiEbeanServer) database; setServerConfig(server.getServerConfig()); } - /** - * Set the DatabaseConfig to use. Typically this is not called explicitly. - */ @Override public void setServerConfig(DatabaseConfig config) { if (this.databaseConfig == null) { @@ -173,7 +138,6 @@ public class DefaultDbMigration implements DbMigration { if (constraintNaming == null) { this.constraintNaming = databaseConfig.getConstraintNaming(); } - Properties properties = config.getProperties(); if (properties != null) { PropertiesWrapper props = new PropertiesWrapper("ebean", config.getName(), properties, null); @@ -242,7 +206,7 @@ public class DefaultDbMigration implements DbMigration { @Override public void setPlatform(Platform platform) { vanillaPlatform = true; - setPlatform(getPlatform(platform)); + setPlatform(platform(platform)); } /** @@ -267,7 +231,7 @@ public class DefaultDbMigration implements DbMigration { @Override public void addPlatform(Platform platform, String prefix) { - platforms.add(new Pair(getPlatform(platform), prefix)); + platforms.add(new Pair(platform(platform), prefix)); } @Override @@ -402,7 +366,6 @@ public class DefaultDbMigration implements DbMigration { *

*/ private void generateExtraDdl(File migrationDir, DatabasePlatform dbPlatform, boolean tablePartitioning) throws IOException { - if (dbPlatform != null) { if (tablePartitioning && includeBuiltInPartitioning) { generateExtraDdlFor(migrationDir, dbPlatform, ExtraDdlXmlReader.readBuiltinTablePartitioning()); @@ -427,10 +390,8 @@ public class DefaultDbMigration implements DbMigration { * Write (or override) the "repeatable" migration script. */ private void writeExtraDdl(File migrationDir, DdlScript script) throws IOException { - String fullName = repeatableMigrationName(script.isInit(), script.getName()); logger.debug("writing repeatable script {}", fullName); - File file = new File(migrationDir, fullName); try (FileWriter writer = new FileWriter(file)) { writer.write(script.getValue()); @@ -478,12 +439,10 @@ public class DefaultDbMigration implements DbMigration { * Generate the diff migration. */ private String generateDiff(Request request) throws IOException { - List pendingDrops = request.getPendingDrops(); if (!pendingDrops.isEmpty()) { logInfo("Pending un-applied drops in versions %s", pendingDrops); } - Migration migration = request.createDiffMigration(); if (migration == null) { logInfo("no changes detected - no migration written", null); @@ -498,11 +457,8 @@ public class DefaultDbMigration implements DbMigration { * Generate the migration based on the pendingDrops from a prior version. */ private String generatePendingDrop(Request request, String pendingVersion) throws IOException { - Migration migration = request.migrationForPendingDrop(pendingVersion); - String version = generateMigration(request, migration, pendingVersion); - List pendingDrops = request.getPendingDrops(); if (!pendingDrops.isEmpty()) { logInfo("... remaining pending un-applied drops in versions %s", pendingDrops); @@ -527,12 +483,12 @@ public class DefaultDbMigration implements DbMigration { this.initMigration = initMigration; this.currentModel = new CurrentModel(server, constraintNaming); this.current = currentModel.read(); - this.migrationDir = getMigrationDirectory(initMigration); + this.migrationDir = migrationDirectory(initMigration); if (initMigration) { this.modelDir = null; this.migrated = new ModelContainer(); } else { - this.modelDir = getModelDirectory(migrationDir); + this.modelDir = modelDirectory(migrationDir); MigrationModel migrationModel = new MigrationModel(modelDir, modelSuffix); this.migrated = migrationModel.read(false); } @@ -547,8 +503,8 @@ public class DefaultDbMigration implements DbMigration { */ String nextVersion() { // always read the next version using the main migration directory (not dbinit) - File migDirectory = getMigrationDirectory(false); - File modelDir = getModelDirectory(migDirectory); + File migDirectory = migrationDirectory(false); + File modelDir = modelDirectory(migDirectory); return LastMigration.nextVersion(migDirectory, modelDir, initMigration); } @@ -556,9 +512,7 @@ public class DefaultDbMigration implements DbMigration { * Return the migration for the pending drops for a given version. */ Migration migrationForPendingDrop(String pendingVersion) { - Migration migration = migrated.migrationForPendingDrop(pendingVersion); - // register any remaining pending drops migrated.registerPendingHistoryDropColumns(current); return migration; @@ -582,9 +536,7 @@ public class DefaultDbMigration implements DbMigration { } private String generateMigration(Request request, Migration dbMigration, String dropsFor) throws IOException { - - String fullVersion = getFullVersion(request.nextVersion(), dropsFor); - + String fullVersion = fullVersion(request.nextVersion(), dropsFor); logInfo("generating migration:%s", fullVersion); if (!request.initMigration && !writeMigrationXml(dbMigration, request.modelDir, fullVersion)) { logError("migration already exists, not generating DDL"); @@ -621,15 +573,14 @@ public class DefaultDbMigration implements DbMigration { *

* The full version can contain a comment suffix after a "__" double underscore. */ - private String getFullVersion(String nextVersion, String dropsFor) { - - String version = getVersion(); + private String fullVersion(String nextVersion, String dropsFor) { + String version = version(); if (version == null) { version = (nextVersion != null) ? nextVersion : initialVersion; } String fullVersion = applyPrefix + version; - String name = getName(); + String name = name(); if (name != null) { fullVersion += "__" + toUnderScore(name); @@ -724,7 +675,7 @@ public class DefaultDbMigration implements DbMigration { * FlywayDb so each developer sets a unique version so that the migration script * generated is unique (typically just prior to being submitted as a merge request). */ - private String getVersion() { + private String version() { String envVersion = readEnvironment("ddl.migration.version"); if (!isEmpty(envVersion)) { return envVersion.trim(); @@ -744,7 +695,7 @@ public class DefaultDbMigration implements DbMigration { * is a short description of the feature. *

*/ - private String getName() { + private String name() { String envName = readEnvironment("ddl.migration.name"); if (!isEmpty(envName)) { return envName.trim(); @@ -773,21 +724,21 @@ public class DefaultDbMigration implements DbMigration { /** * Return the main migration directory. */ - File getMigrationDirectory() { - return getMigrationDirectory(false); + File migrationDirectory() { + return migrationDirectory(false); } /** * Return the file path to write the xml and sql to. */ - File getMigrationDirectory(boolean initMigration) { + File migrationDirectory(boolean initMigration) { // path to src/main/resources in typical maven project File resourceRootDir = new File(pathToResources); if (!resourceRootDir.exists()) { String msg = String.format("Error - path to resources %s does not exist. Absolute path is %s", pathToResources, resourceRootDir.getAbsolutePath()); throw new UnknownResourcePathException(msg); } - String resourcePath = getMigrationPath(initMigration); + String resourcePath = migrationPath(initMigration); // expect to be a path to something like - src/main/resources/dbmigration File path = new File(resourceRootDir, resourcePath); if (!path.exists()) { @@ -798,14 +749,14 @@ public class DefaultDbMigration implements DbMigration { return path; } - private String getMigrationPath(boolean initMigration) { + private String migrationPath(boolean initMigration) { return initMigration ? migrationInitPath : migrationPath; } /** * Return the model directory (relative to the migration directory). */ - private File getModelDirectory(File migrationDirectory) { + private File modelDirectory(File migrationDirectory) { if (modelPath == null || modelPath.isEmpty()) { return migrationDirectory; } @@ -819,7 +770,7 @@ public class DefaultDbMigration implements DbMigration { /** * Return the DatabasePlatform given the platform key. */ - protected DatabasePlatform getPlatform(Platform platform) { + protected DatabasePlatform platform(Platform platform) { switch (platform) { case H2: return new H2Platform(); diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/LastMigration.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/LastMigration.java index 07a80add9..17ac65aa9 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/LastMigration.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/LastMigration.java @@ -18,7 +18,7 @@ class LastMigration { private static final String MODEL_XML = ".model.xml"; /** - * Return the next migation version given the migration directory. + * Return the next migration version given the migration directory. */ static String nextVersion(File migDir, File modelDir, boolean initMigration) { String last = lastVersion(migDir, modelDir); diff --git a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationDropHistoryTest.java b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationDropHistoryTest.java index db91e14c9..320406a7c 100644 --- a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationDropHistoryTest.java +++ b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationDropHistoryTest.java @@ -54,8 +54,8 @@ public class DbMigrationDropHistoryTest { migration.setServer(server); // First, we clean up the output-directory - assertThat(migration.getMigrationDirectory().getAbsolutePath()).contains("migrationtest-history"); - Files.walk(migration.getMigrationDirectory().toPath()) + assertThat(migration.migrationDirectory().getAbsolutePath()).contains("migrationtest-history"); + Files.walk(migration.migrationDirectory().toPath()) .filter(Files::isRegularFile).map(Path::toFile).forEach(File::delete); // then we generate migration scripts for v1_0 diff --git a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java index 0063a75cf..befead534 100644 --- a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java +++ b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java @@ -70,8 +70,8 @@ public class DbMigrationGenerateTest { migration.setServer(server); // First, we clean up the output-directory - assertThat(migration.getMigrationDirectory().getAbsolutePath()).contains("migrationtest"); - Files.walk(migration.getMigrationDirectory().toPath()) + assertThat(migration.migrationDirectory().getAbsolutePath()).contains("migrationtest"); + Files.walk(migration.migrationDirectory().toPath()) .filter(Files::isRegularFile).map(Path::toFile).forEach(File::delete); // then we generate migration scripts for v1_0