From f26d2c8263c0a1cc23a1b37fe3335b84626dd835 Mon Sep 17 00:00:00 2001 From: Gabor Toth Date: Thu, 30 Jun 2022 17:16:58 +0200 Subject: [PATCH 01/12] 2863: autoIncrement issue fixed: MySQL,H2,HSQLDB,MariaDB. --- .../java/liquibase/database/AbstractJdbcDatabase.java | 11 +++++++++-- .../core/AddAutoIncrementGeneratorMySQL.java | 8 ++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java index 1487dbfaed8..b0af1e7e168 100644 --- a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java @@ -581,14 +581,21 @@ public String getAutoIncrementClause(final BigInteger startWith, final BigIntege if (generateStartWith || generateIncrementBy) { autoIncrementClause += getAutoIncrementOpening(); - + if(getShortName().equals("h2")) { + autoIncrementClause+= " START WITH "; + } if (generateStartWith) { autoIncrementClause += String.format(getAutoIncrementStartWithClause(), (startWith == null) ? defaultAutoIncrementStartWith : startWith); } if (generateIncrementBy) { if (generateStartWith) { - autoIncrementClause += ", "; + if(getShortName().equals("h2")) + autoIncrementClause += " INCREMENT BY "; + else{ + if(!getShortName().equals("hsqldb")) + autoIncrementClause += ", "; + } } autoIncrementClause += String.format(getAutoIncrementByClause(), (incrementBy == null) ? defaultAutoIncrementBy : incrementBy); diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java index 7cf34d86843..a5ad170a929 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java @@ -2,6 +2,7 @@ import liquibase.database.Database; import liquibase.database.core.MySQLDatabase; +import liquibase.exception.ValidationErrors; import liquibase.sql.Sql; import liquibase.sql.UnparsedSql; import liquibase.sqlgenerator.SqlGeneratorChain; @@ -40,6 +41,13 @@ public Sql[] generateSql(final AddAutoIncrementStatement statement, Database dat return sql; } + @Override + public ValidationErrors validate(AddAutoIncrementStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { + ValidationErrors validationErrors = super.validate(statement, database, sqlGeneratorChain); + validationErrors.checkDisallowedField("incrementBy",statement.getIncrementBy(),database,MySQLDatabase.class); + return validationErrors; + } + private Sql[] concact(Sql[] origSql, UnparsedSql unparsedSql) { Sql[] changedSql = new Sql[origSql.length+1]; System.arraycopy(origSql, 0, changedSql, 0, origSql.length); From 92154879b6fea761cb9d65b27e87c7dc0d698b9a Mon Sep 17 00:00:00 2001 From: Gabor Toth Date: Thu, 30 Jun 2022 22:56:15 +0200 Subject: [PATCH 02/12] 2863: refact H2,HSQL auto increment clause, abstract hsql and h2 database class --- .../database/AbstractHsqlAndH2Database.java | 24 ++++++++++ .../database/AbstractJdbcDatabase.java | 46 +++++++++---------- .../liquibase/database/core/H2Database.java | 5 +- .../liquibase/database/core/HsqlDatabase.java | 6 ++- .../core/CreateTableGeneratorTest.java | 4 +- 5 files changed, 56 insertions(+), 29 deletions(-) create mode 100644 liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java diff --git a/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java b/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java new file mode 100644 index 00000000000..e56a612edec --- /dev/null +++ b/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java @@ -0,0 +1,24 @@ +package liquibase.database; + +import java.math.BigInteger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public abstract class AbstractHsqlAndH2Database extends AbstractJdbcDatabase { + + private final Pattern pattern = Pattern.compile("(\\d+)(,).*(\\d+)"); + + @Override + public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) { + StringBuilder autoIncrement = new StringBuilder(super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull)); + if(startWith != null && incrementBy != null) { + Matcher m = pattern.matcher(autoIncrement.toString()); + if(m.find()) { + String[] splittedClause = autoIncrement.toString().split("\\("); + autoIncrement = new StringBuilder(); + autoIncrement.append(splittedClause[0]).append("(START WITH "+startWith.longValue()).append(" INCREMENT BY "+incrementBy.longValue()+")"); + } + } + return autoIncrement.toString(); + } +} diff --git a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java index b0af1e7e168..e21323bf9b5 100644 --- a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java @@ -310,7 +310,7 @@ public String correctObjectName(final String objectName, final Class objectType) { return objectType.equals(Catalog.class) || objectType.equals(Schema.class); } + private CatalogAndSchema.CatalogAndSchemaCase preserveCaseIfRequested() { if (Boolean.TRUE.equals(GlobalConfiguration.PRESERVE_SCHEMA_CASE.getCurrentValue())) { - return CatalogAndSchema.CatalogAndSchemaCase.ORIGINAL_CASE; + return CatalogAndSchema.CatalogAndSchemaCase.ORIGINAL_CASE; } return getSchemaAndCatalogCase(); } @@ -368,6 +369,7 @@ public void setDefaultSchemaName(final String schemaName) { /** * Overwrite this method to get the default schema name for the connection. * If you only need to change the statement that obtains the current schema then override + * * @see AbstractJdbcDatabase#getConnectionSchemaNameCallStatement() */ protected String getConnectionSchemaName() { @@ -395,9 +397,10 @@ protected String getConnectionSchemaName() { * Used to obtain the connection schema name through a statement * Override this method to change the statement. * Only override this if getConnectionSchemaName is left unchanges or is using this method. + * * @see AbstractJdbcDatabase#getConnectionSchemaName() */ - protected SqlStatement getConnectionSchemaNameCallStatement(){ + protected SqlStatement getConnectionSchemaNameCallStatement() { return new RawCallStatement("call current_schema"); } @@ -485,7 +488,7 @@ public String getDateLiteral(final Date date) { return getTimeLiteral(((java.sql.Time) date)); } else if (date instanceof java.sql.Timestamp) { return getDateTimeLiteral(((java.sql.Timestamp) date)); - } else if(date instanceof java.util.Date) { + } else if (date instanceof java.util.Date) { return getDateTimeLiteral(new java.sql.Timestamp(date.getTime())); } else { throw new RuntimeException("Unexpected type: " + date.getClass().getName()); @@ -581,21 +584,14 @@ public String getAutoIncrementClause(final BigInteger startWith, final BigIntege if (generateStartWith || generateIncrementBy) { autoIncrementClause += getAutoIncrementOpening(); - if(getShortName().equals("h2")) { - autoIncrementClause+= " START WITH "; - } if (generateStartWith) { autoIncrementClause += String.format(getAutoIncrementStartWithClause(), (startWith == null) ? defaultAutoIncrementStartWith : startWith); } if (generateIncrementBy) { if (generateStartWith) { - if(getShortName().equals("h2")) - autoIncrementClause += " INCREMENT BY "; - else{ - if(!getShortName().equals("hsqldb")) - autoIncrementClause += ", "; - } + autoIncrementClause += ", "; + } autoIncrementClause += String.format(getAutoIncrementByClause(), (incrementBy == null) ? defaultAutoIncrementBy : incrementBy); @@ -765,8 +761,8 @@ public boolean isReservedWord(final String string) { } /* - * Check if given string starts with numeric values that may cause problems and should be escaped. - */ + * Check if given string starts with numeric values that may cause problems and should be escaped. + */ protected boolean startsWithNumeric(final String objectName) { return STARTS_WITH_NUMBER_PATTERN.matcher(objectName).matches(); } @@ -804,7 +800,7 @@ public void dropDatabaseObjects(final CatalogAndSchema schemaToDrop) throws Liqu final long changeSetStarted = System.currentTimeMillis(); CompareControl compareControl = new CompareControl( - new CompareControl.SchemaComparison[] { + new CompareControl.SchemaComparison[]{ new CompareControl.SchemaComparison( CatalogAndSchema.DEFAULT, schemaToDrop)}, @@ -856,7 +852,7 @@ public void dropDatabaseObjects(final CatalogAndSchema schemaToDrop) throws Liqu @Override public boolean supportsDropTableCascadeConstraints() { return ((this instanceof SQLiteDatabase) || (this instanceof SybaseDatabase) || (this instanceof - SybaseASADatabase) || (this instanceof PostgresDatabase) || (this instanceof OracleDatabase)); + SybaseASADatabase) || (this instanceof PostgresDatabase) || (this instanceof OracleDatabase)); } @Override @@ -865,7 +861,7 @@ public boolean isSystemObject(final DatabaseObject example) { return false; } if ((example.getSchema() != null) && (example.getSchema().getName() != null) && "information_schema" - .equalsIgnoreCase(example.getSchema().getName())) { + .equalsIgnoreCase(example.getSchema().getName())) { return true; } if ((example instanceof Table) && getSystemTables().contains(example.getName())) { @@ -1271,7 +1267,6 @@ public void setAutoCommit(final boolean b) throws DatabaseException { * Default implementation, just look for "local" IPs. If the database returns a null URL we return false since we don't know it's safe to run the update. * * @throws liquibase.exception.DatabaseException - * */ @Override public boolean isSafeToRunUpdate() throws DatabaseException { @@ -1311,7 +1306,7 @@ public void execute(final SqlStatement[] statements, final List sqlV Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", this).execute(statement, sqlVisitors); } catch (DatabaseException e) { if (statement.continueOnError()) { - Scope.getCurrentScope().getLog(getClass()).severe("Error executing statement '"+statement.toString()+"', but continuing", e); + Scope.getCurrentScope().getLog(getClass()).severe("Error executing statement '" + statement.toString() + "', but continuing", e); } else { throw e; } @@ -1321,7 +1316,7 @@ public void execute(final SqlStatement[] statements, final List sqlV @Override public void saveStatements(final Change change, final List sqlVisitors, final Writer writer) throws - IOException { + IOException { SqlStatement[] statements = change.generateStatements(this); for (SqlStatement statement : statements) { for (Sql sql : SqlGeneratorFactory.getInstance().generateSql(statement, this)) { @@ -1478,7 +1473,7 @@ public String generateDatabaseFunctionValue(final DatabaseFunction databaseFunct if (sequenceNextValueFunction.contains("'")) { /* For PostgreSQL, the quotes around dangerous identifiers (e.g. mixed-case) need to stay in place, * or else PostgreSQL will not be able to find the sequence. */ - if (! (this instanceof PostgresDatabase)) { + if (!(this instanceof PostgresDatabase)) { sequenceName = sequenceName.replace("\"", ""); } } @@ -1497,7 +1492,7 @@ public String generateDatabaseFunctionValue(final DatabaseFunction databaseFunct if (sequenceCurrentValueFunction.contains("'")) { /* For PostgreSQL, the quotes around dangerous identifiers (e.g. mixed-case) need to stay in place, * or else PostgreSQL will not be able to find the sequence. */ - if (! (this instanceof PostgresDatabase)) { + if (!(this instanceof PostgresDatabase)) { sequenceName = sequenceName.replace("\"", ""); } } @@ -1579,7 +1574,7 @@ public boolean supportsPrimaryKeyNames() { } @Override - public String getSystemSchema(){ + public String getSystemSchema() { return "information_schema"; } @@ -1650,7 +1645,7 @@ public boolean supportsBatchUpdates() throws DatabaseException { if (connection instanceof OfflineConnection) { return false; } else if (connection instanceof JdbcConnection) { - return ((JdbcConnection)getConnection()).supportsBatchUpdates(); + return ((JdbcConnection) getConnection()).supportsBatchUpdates(); } else { // Normally, the connection can only be one of the two above types. But if, for whatever reason, it is // not, let's err on the safe side. @@ -1671,6 +1666,7 @@ public boolean requiresExplicitNullForColumns() { /** * This logic is used when db support catalogs + * * @return UPPER_CASE by default */ @Override diff --git a/liquibase-core/src/main/java/liquibase/database/core/H2Database.java b/liquibase-core/src/main/java/liquibase/database/core/H2Database.java index 3d9ee5518ad..f9672b310a9 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/H2Database.java +++ b/liquibase-core/src/main/java/liquibase/database/core/H2Database.java @@ -2,6 +2,7 @@ import liquibase.CatalogAndSchema; import liquibase.Scope; +import liquibase.database.AbstractHsqlAndH2Database; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; import liquibase.database.OfflineConnection; @@ -14,6 +15,7 @@ import liquibase.util.JdbcUtil; import java.lang.reflect.Method; +import java.math.BigInteger; import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; @@ -26,7 +28,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -public class H2Database extends AbstractJdbcDatabase { +public class H2Database extends AbstractHsqlAndH2Database { private static String START_CONCAT = "CONCAT("; private static String END_CONCAT = ")"; @@ -322,6 +324,7 @@ public boolean supportsDropTableCascadeConstraints() { return true; } + @Override public void setConnection(DatabaseConnection conn) { Connection sqlConn = null; diff --git a/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java b/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java index ccf56789287..6502839db23 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java @@ -1,5 +1,6 @@ package liquibase.database.core; +import liquibase.database.AbstractHsqlAndH2Database; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; import liquibase.database.ObjectQuotingStrategy; @@ -13,10 +14,13 @@ import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static liquibase.statement.DatabaseFunction.CURRENT_DATE_TIME_PLACE_HOLDER; -public class HsqlDatabase extends AbstractJdbcDatabase { +public class HsqlDatabase extends AbstractHsqlAndH2Database { + private static final Map> SUPPORTED_DEFAULT_VALUE_COMPUTED_MAP; private static String START_CONCAT = "CONCAT("; private static String END_CONCAT = ")"; diff --git a/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java b/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java index 305c810ef76..051bae4a8ee 100644 --- a/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java +++ b/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java @@ -558,7 +558,7 @@ public void testAutoIncrementStartWithIncrementByH2Database() throws Exception { Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null); - assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (0, 10))", generatedSql[0].toSql()); + assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 0 INCREMENT BY 10))", generatedSql[0].toSql()); } } } @@ -612,7 +612,7 @@ public void testAutoIncrementStartWithIncrementByHsqlDatabase() throws Exception Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null); - assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1, INCREMENT BY 10))", generatedSql[0].toSql()); + assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1 INCREMENT BY 10))", generatedSql[0].toSql()); } } } From 9f80bc00f3d85c30d381b48e3c3d8620e07863a1 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Thu, 7 Jul 2022 13:43:35 -0500 Subject: [PATCH 03/12] Fixing autoincrement logic for h2, hsql, and mysql --- .../database/AbstractHsqlAndH2Database.java | 24 ------------------- .../liquibase/database/core/H2Database.java | 16 ++++++------- .../liquibase/database/core/HsqlDatabase.java | 11 +++++---- .../core/AddAutoIncrementGeneratorMySQL.java | 4 ++-- .../core/CreateTableGeneratorTest.java | 2 +- 5 files changed, 17 insertions(+), 40 deletions(-) delete mode 100644 liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java diff --git a/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java b/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java deleted file mode 100644 index e56a612edec..00000000000 --- a/liquibase-core/src/main/java/liquibase/database/AbstractHsqlAndH2Database.java +++ /dev/null @@ -1,24 +0,0 @@ -package liquibase.database; - -import java.math.BigInteger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -public abstract class AbstractHsqlAndH2Database extends AbstractJdbcDatabase { - - private final Pattern pattern = Pattern.compile("(\\d+)(,).*(\\d+)"); - - @Override - public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) { - StringBuilder autoIncrement = new StringBuilder(super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull)); - if(startWith != null && incrementBy != null) { - Matcher m = pattern.matcher(autoIncrement.toString()); - if(m.find()) { - String[] splittedClause = autoIncrement.toString().split("\\("); - autoIncrement = new StringBuilder(); - autoIncrement.append(splittedClause[0]).append("(START WITH "+startWith.longValue()).append(" INCREMENT BY "+incrementBy.longValue()+")"); - } - } - return autoIncrement.toString(); - } -} diff --git a/liquibase-core/src/main/java/liquibase/database/core/H2Database.java b/liquibase-core/src/main/java/liquibase/database/core/H2Database.java index f9672b310a9..fe117dc0054 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/H2Database.java +++ b/liquibase-core/src/main/java/liquibase/database/core/H2Database.java @@ -2,7 +2,6 @@ import liquibase.CatalogAndSchema; import liquibase.Scope; -import liquibase.database.AbstractHsqlAndH2Database; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; import liquibase.database.OfflineConnection; @@ -28,7 +27,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -public class H2Database extends AbstractHsqlAndH2Database { +public class H2Database extends AbstractJdbcDatabase { private static String START_CONCAT = "CONCAT("; private static String END_CONCAT = ")"; @@ -305,13 +304,13 @@ protected String getAutoIncrementClause() { } @Override - protected String getAutoIncrementStartWithClause() { - return "%d"; - } + public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) { + final String clause = super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull); + if (clause.startsWith("AUTO_INCREMENT")) { + return clause; + } - @Override - protected String getAutoIncrementByClause() { - return "%d"; + return clause.replace(",", ""); //h2 doesn't use commas between the values } @Override @@ -324,7 +323,6 @@ public boolean supportsDropTableCascadeConstraints() { return true; } - @Override public void setConnection(DatabaseConnection conn) { Connection sqlConn = null; diff --git a/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java b/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java index 6502839db23..30318dde573 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java @@ -1,6 +1,5 @@ package liquibase.database.core; -import liquibase.database.AbstractHsqlAndH2Database; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; import liquibase.database.ObjectQuotingStrategy; @@ -14,12 +13,10 @@ import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.*; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import static liquibase.statement.DatabaseFunction.CURRENT_DATE_TIME_PLACE_HOLDER; -public class HsqlDatabase extends AbstractHsqlAndH2Database { +public class HsqlDatabase extends AbstractJdbcDatabase { private static final Map> SUPPORTED_DEFAULT_VALUE_COMPUTED_MAP; private static String START_CONCAT = "CONCAT("; @@ -516,4 +513,10 @@ public int getMaxFractionalDigitsForTimestamp() { // this value is derived from tests. return 9; } + + @Override + public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) { + final String clause = super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull); + return clause.replace(",", ""); //sql doesn't use commas between the values + } } diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java index a5ad170a929..2efb88f7ed4 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java @@ -44,7 +44,7 @@ public Sql[] generateSql(final AddAutoIncrementStatement statement, Database dat @Override public ValidationErrors validate(AddAutoIncrementStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { ValidationErrors validationErrors = super.validate(statement, database, sqlGeneratorChain); - validationErrors.checkDisallowedField("incrementBy",statement.getIncrementBy(),database,MySQLDatabase.class); + validationErrors.checkDisallowedField("incrementBy", statement.getIncrementBy(), database, MySQLDatabase.class); return validationErrors; } @@ -59,4 +59,4 @@ private Sql[] concact(Sql[] origSql, UnparsedSql unparsedSql) { private DatabaseObject getAffectedTable(AddAutoIncrementStatement statement) { return new Table().setName(statement.getTableName()).setSchema(new Schema(statement.getCatalogName(), statement.getSchemaName())); } -} \ No newline at end of file +} diff --git a/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java b/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java index 051bae4a8ee..639275c234c 100644 --- a/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java +++ b/liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java @@ -540,7 +540,7 @@ public void testAutoIncrementStartWithH2Database() throws Exception { Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null); - assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (0))", generatedSql[0].toSql()); + assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 0))", generatedSql[0].toSql()); } } } From ee29021e03d5048d07c56f25cf25dd2dbcf8ada9 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Thu, 7 Jul 2022 13:57:20 -0500 Subject: [PATCH 04/12] Added validation warning for incrementBy on mysql --- .../liquibase/sqlgenerator/core/CreateTableGenerator.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java index 42f023382cc..c3dfe160cab 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java @@ -43,6 +43,12 @@ public ValidationErrors validate(CreateTableStatement createTableStatement, Data ValidationErrors validationErrors = new ValidationErrors(); validationErrors.checkRequiredField("tableName", createTableStatement.getTableName()); validationErrors.checkRequiredField("columns", createTableStatement.getColumns()); + + if (createTableStatement.getAutoIncrementConstraints() != null) { + for (AutoIncrementConstraint constraint : createTableStatement.getAutoIncrementConstraints()) { + validationErrors.checkDisallowedField("incrementBy", constraint.getIncrementBy(), database, MySQLDatabase.class); + } + } return validationErrors; } From c5158884d8f30f4cf39b411b5f01b2d93733ba7c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Jul 2022 02:26:08 +0000 Subject: [PATCH 05/12] Bump maven-assembly-plugin from 3.3.0 to 3.4.1 Bumps [maven-assembly-plugin](https://github.com/apache/maven-assembly-plugin) from 3.3.0 to 3.4.1. - [Release notes](https://github.com/apache/maven-assembly-plugin/releases) - [Commits](https://github.com/apache/maven-assembly-plugin/compare/maven-assembly-plugin-3.3.0...maven-assembly-plugin-3.4.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-assembly-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- liquibase-dist/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liquibase-dist/pom.xml b/liquibase-dist/pom.xml index f4aa8257026..35cf4d11d1f 100644 --- a/liquibase-dist/pom.xml +++ b/liquibase-dist/pom.xml @@ -224,7 +224,7 @@ maven-assembly-plugin - 3.3.0 + 3.4.1 liquibase-${project.version} false From dd96535c55450378057207a4a762f7d318732f88 Mon Sep 17 00:00:00 2001 From: Sajad Mehrabi Date: Wed, 13 Jul 2022 14:22:59 +0430 Subject: [PATCH 06/12] Fixed test failing in different timezones --- .../parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy index a42e92b307c..0dfeed9bc90 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy @@ -551,7 +551,7 @@ public class YamlChangeLogParser_RealFile_Test extends Specification { ((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[3].defaultValueComputed.toString() == "average_size()" ((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[4].name == "new_col_datetime" - new ISODateFormat().format(new java.sql.Timestamp(((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[4].defaultValueDate.time)).matches(/2014-12-\d+T\d+:15:33/) //timezones shift actual value around + new ISODateFormat().format(new java.sql.Timestamp(((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[4].defaultValueDate.time)).matches(/2014-12-\d+T\d+:\d+:33/) //timezones shift actual value around ((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[5].name == "new_col_seq" ((AddColumnChange) changeLog.getChangeSet(path, "nvoxland", "different object types for column").changes[1]).columns[5].defaultValueSequenceNext.toString() == "seq_test" From 11436896e67d367c6423fe0635abf0b3476f4426 Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Tue, 19 Jul 2022 12:14:38 -0500 Subject: [PATCH 07/12] error when JAVA_HOME is set to invalid value (DAT-10545) (#3074) Throw an error and fail to start Liquibase if the JAVA_HOME environment variable is set and the path it points to does not exist. --- liquibase-dist/src/main/archive/liquibase | 5 +++++ liquibase-dist/src/main/archive/liquibase.bat | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/liquibase-dist/src/main/archive/liquibase b/liquibase-dist/src/main/archive/liquibase index b1e10e39021..088a19e86ec 100644 --- a/liquibase-dist/src/main/archive/liquibase +++ b/liquibase-dist/src/main/archive/liquibase @@ -30,6 +30,11 @@ if [ -z "${JAVA_HOME}" ]; then elif [ -d "${LIQUIBASE_HOME}/.install4j/jre.bundle/Contents/Home" ]; then JAVA_HOME="${LIQUIBASE_HOME}/.install4j/jre.bundle/Contents/Home" fi +else + if [ ! -x "$JAVA_HOME" ] ; then + echo "ERROR: The JAVA_HOME environment variable is not defined correctly, so Liquibase cannot be started. JAVA_HOME is set to \"$JAVA_HOME\" and it does not exist." >&2 + exit 1 + fi fi if [ -z "${JAVA_HOME}" ]; then diff --git a/liquibase-dist/src/main/archive/liquibase.bat b/liquibase-dist/src/main/archive/liquibase.bat index 4ad3e373c64..99b54cb3ac9 100644 --- a/liquibase-dist/src/main/archive/liquibase.bat +++ b/liquibase-dist/src/main/archive/liquibase.bat @@ -17,6 +17,11 @@ if exist "%LIQUIBASE_HOME%\jre" if "%JAVA_HOME%"=="" ( set JAVA_HOME=%LIQUIBASE_HOME%\jre ) +if NOT "%JAVA_HOME%" == "" if not exist "%JAVA_HOME%" ( + echo ERROR: The JAVA_HOME environment variable is not defined correctly, so Liquibase cannot be started. JAVA_HOME is set to "%JAVA_HOME%" and it does not exist. >&2 + exit /B 1 +) + rem special characters may be lost setlocal DISABLEDELAYEDEXPANSION From b29e9faec97d530de7b78f1cf4043cbe05ec4f32 Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Wed, 20 Jul 2022 08:22:28 -0500 Subject: [PATCH 08/12] include pro DatabaseObject's in the permitted list for filtering with includeObjects/excludeObjects in generateChangeLog (#3068) --- .../output/StandardObjectChangeFilter.java | 27 ++++++----- .../command/generateChangelog.test.groovy | 45 +++++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/diff/output/StandardObjectChangeFilter.java b/liquibase-core/src/main/java/liquibase/diff/output/StandardObjectChangeFilter.java index b27649f6f88..2c8f76477b1 100644 --- a/liquibase-core/src/main/java/liquibase/diff/output/StandardObjectChangeFilter.java +++ b/liquibase-core/src/main/java/liquibase/diff/output/StandardObjectChangeFilter.java @@ -1,14 +1,17 @@ package liquibase.diff.output; +import liquibase.Scope; import liquibase.database.Database; import liquibase.diff.ObjectDifferences; import liquibase.exception.UnexpectedLiquibaseException; +import liquibase.servicelocator.ServiceLocator; import liquibase.structure.DatabaseObject; import liquibase.structure.core.*; import liquibase.util.StringUtil; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.regex.Pattern; /** @@ -24,13 +27,18 @@ */ public class StandardObjectChangeFilter implements ObjectChangeFilter { - private FilterType filterType; + private final FilterType filterType; - private List filters = new ArrayList<>(); + private final List filters = new ArrayList<>(); + private final static List databaseObjects = new ArrayList<>(); private boolean catalogOrSchemaFilter; public StandardObjectChangeFilter(FilterType type, String filter) { this.filterType = type; + if (databaseObjects.isEmpty()) { + ServiceLocator serviceLocator = Scope.getCurrentScope().getServiceLocator(); + databaseObjects.addAll(serviceLocator.findInstances(DatabaseObject.class)); + } parseFilter(filter); } @@ -47,15 +55,14 @@ protected void parseFilter(String filter) { if (split.length == 1) { filters.add(new Filter(null, Pattern.compile(split[0]))); } else { - String className = StringUtil.upperCaseFirst(split[0]); - className = "liquibase.structure.core."+className; - try { - Class clazz = (Class) Class.forName(className); - filters.add(new Filter(clazz, Pattern.compile(split[1]))); - catalogOrSchemaFilter |= "Catalog".equals(className) || "Schema".equals(className); - } catch (ClassNotFoundException e) { - throw new UnexpectedLiquibaseException(e); + String className = split[0]; + Optional databaseObject = databaseObjects.stream().filter(instance -> instance.getClass().getSimpleName().equalsIgnoreCase(className)).findAny(); + if (databaseObject.isPresent()) { + filters.add(new Filter((Class) databaseObject.get().getClass(), Pattern.compile(split[1]))); + } else { + throw new UnexpectedLiquibaseException(className + " not found"); } + catalogOrSchemaFilter |= "Catalog".equals(className) || "Schema".equals(className); } } } diff --git a/liquibase-integration-tests/src/test/resources/liquibase/extension/testing/command/generateChangelog.test.groovy b/liquibase-integration-tests/src/test/resources/liquibase/extension/testing/command/generateChangelog.test.groovy index d546bab97d6..4dd16cafe48 100644 --- a/liquibase-integration-tests/src/test/resources/liquibase/extension/testing/command/generateChangelog.test.groovy +++ b/liquibase-integration-tests/src/test/resources/liquibase/extension/testing/command/generateChangelog.test.groovy @@ -89,6 +89,51 @@ Optional Args: ] } + run "Filtering with includeObjects", { + arguments = [ + url : { it.url }, + username: { it.username }, + password: { it.password }, + changelogFile: "target/test-classes/changelog-test.xml", + includeObjects: "table:FIRSTTABLE" + ] + setup { + cleanResources(SetupCleanResources.CleanupMode.CLEAN_ON_SETUP, "changelog-test.xml") + database = [ + new CreateTableChange( + tableName: "FirstTable", + columns: [ + ColumnConfig.fromName("FirstColumn") + .setType("VARCHAR(255)") + ] + ), + new CreateTableChange( + tableName: "SecondTable", + columns: [ + ColumnConfig.fromName("SecondColumn") + .setType("VARCHAR(255)") + ] + ), + new TagDatabaseChange( + tag: "version_2.0" + ), + new CreateTableChange( + tableName: "liquibaseRunInfo", + columns: [ + ColumnConfig.fromName("timesRan") + .setType("INT") + ] + ), + ] + } + expectedFileContent = [ + "target/test-classes/changelog-test.xml" : [CommandTests.assertContains(" Date: Wed, 20 Jul 2022 15:40:17 +0100 Subject: [PATCH 09/12] CORE-3276 Update to RenameColumnGenerator.generateSql (#783) Update to RenameColumnGenerator.generateSql Mssql fix to follow syntax as per msdn: sp_rename [ @objname = ] 'object_name' , [ @newname = ] 'new_name' [ , [ @objtype = ] 'object_type' ] https://docs.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-rename-transact-sql?view=sql-server-2017 Co-authored-by: Nathan Voxland --- .../java/liquibase/sqlgenerator/core/RenameColumnGenerator.java | 2 +- .../renameColumn/mssql.sql | 2 +- .../liquibase/statementexecute/RenameColumnExecuteTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/RenameColumnGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/RenameColumnGenerator.java index dd0f7ccc8fa..17ff78f505e 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/RenameColumnGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/RenameColumnGenerator.java @@ -38,7 +38,7 @@ public Sql[] generateSql(RenameColumnStatement statement, Database database, Sql String sql; if (database instanceof MSSQLDatabase) { // do no escape the new column name. Otherwise it produce "exec sp_rename '[dbo].[person].[usernae]', '[username]'" - sql = "exec sp_rename '" + database.escapeTableName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName()) + "." + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getOldColumnName()) + "', '" + statement.getNewColumnName() + "'"; + sql = "exec sp_rename '" + database.escapeTableName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName()) + "." + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getOldColumnName()) + "', '" + statement.getNewColumnName() + "', 'COLUMN'"; } else if (database instanceof MySQLDatabase) { sql ="ALTER TABLE " + database.escapeTableName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName()) + " CHANGE " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getOldColumnName()) + " " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getNewColumnName()) + " " + DataTypeFactory.getInstance().fromDescription(statement.getColumnDataType(), database).toDatabaseDataType(database); } else if (database instanceof SybaseDatabase) { diff --git a/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/renameColumn/mssql.sql b/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/renameColumn/mssql.sql index 61ae5adcd6d..eabeb645430 100644 --- a/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/renameColumn/mssql.sql +++ b/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/renameColumn/mssql.sql @@ -2,4 +2,4 @@ -- Change Parameter: newColumnName=full_name -- Change Parameter: oldColumnName=name -- Change Parameter: tableName=person -exec sp_rename 'person.name', 'full_name'; +exec sp_rename 'person.name', 'full_name', 'COLUMN'; diff --git a/liquibase-integration-tests/src/test/java/liquibase/statementexecute/RenameColumnExecuteTest.java b/liquibase-integration-tests/src/test/java/liquibase/statementexecute/RenameColumnExecuteTest.java index 115de3401f8..aebc561f43c 100644 --- a/liquibase-integration-tests/src/test/java/liquibase/statementexecute/RenameColumnExecuteTest.java +++ b/liquibase-integration-tests/src/test/java/liquibase/statementexecute/RenameColumnExecuteTest.java @@ -50,7 +50,7 @@ public void noSchema() throws Exception { assertCorrect("alter table table_name alter column column_name rename to new_name", H2Database.class, HsqlDatabase.class); assertCorrect("alter table table_name alter column column_name to new_name", FirebirdDatabase.class); assertCorrect("alter table table_name change column_name new_name int", MySQLDatabase.class, MariaDBDatabase.class); - assertCorrect("exec sp_rename '[table_name].[column_name]', 'new_name'", MSSQLDatabase.class); + assertCorrect("exec sp_rename '[table_name].[column_name]', 'new_name', 'COLUMN'", MSSQLDatabase.class); assertCorrect("exec sp_rename 'table_name.column_name', 'new_name'", SybaseDatabase.class); assertCorrect("alter table [table_name] rename column_name to new_name",SybaseASADatabase.class); assertCorrectOnRest("alter table [table_name] rename column [column_name] to [new_name]"); From c72e682888eab054b5c56ad7fa2138710d82baf7 Mon Sep 17 00:00:00 2001 From: Zorglube <630192+zorglube@users.noreply.github.com> Date: Wed, 20 Jul 2022 16:44:08 +0200 Subject: [PATCH 10/12] Add Closeable/AutoCloseable on Database & interface (#2990) * Add AutoCloseable on Database interface * Adapt close() implementation into AbstractJdbcDatabase * Handle the eventuality of no connection. * Use `try with resources` into unit test Co-authored-by: Bertrand Moreau Co-authored-by: Nathan Voxland --- .../database/AbstractJdbcDatabase.java | 61 +++++++--------- .../java/liquibase/database/Database.java | 24 ++++--- .../database/DatabaseConnection.java | 8 +-- .../liquibase/database/OfflineConnection.java | 1 - .../database/core/DerbyDatabase.java | 8 ++- .../database/AbstractJdbcDatabaseTest.java | 25 ++++--- .../ConnectionServiceFactoryTest.java | 8 +-- .../core/AbstractDb2DatabaseTest.java | 18 +++-- .../database/core/DB2DatabaseTest.java | 13 ++-- .../database/core/DB2zDatabaseTest.java | 13 ++-- .../database/core/DerbyDatabaseTest.java | 13 ++-- .../database/core/MSSQLDatabaseTest.java | 59 ++++++++------- .../database/core/MySQLDatabaseTest.java | 40 ++++++----- .../database/core/OracleDatabaseTest.java | 72 +++++++++---------- .../database/core/PostgresDatabaseTest.java | 23 +++--- .../core/UnsupportedDatabaseTest.java | 20 +++--- 16 files changed, 215 insertions(+), 191 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java index e21323bf9b5..368c4946c49 100644 --- a/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java @@ -1,6 +1,24 @@ package liquibase.database; +import static liquibase.util.StringUtil.join; +import java.io.IOException; +import java.io.Writer; +import java.math.BigInteger; +import java.sql.SQLException; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; import liquibase.CatalogAndSchema; +import liquibase.GlobalConfiguration; import liquibase.Scope; import liquibase.change.Change; import liquibase.change.core.DropTableChange; @@ -9,7 +27,6 @@ import liquibase.changelog.DatabaseChangeLog; import liquibase.changelog.RanChangeSet; import liquibase.changelog.StandardChangeLogHistoryService; -import liquibase.GlobalConfiguration; import liquibase.configuration.ConfiguredValue; import liquibase.database.core.OracleDatabase; import liquibase.database.core.PostgresDatabase; @@ -60,25 +77,6 @@ import liquibase.util.StreamUtil; import liquibase.util.StringUtil; -import java.io.IOException; -import java.io.Writer; -import java.math.BigInteger; -import java.sql.SQLException; -import java.text.ParseException; -import java.text.SimpleDateFormat; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import java.util.regex.Pattern; - -import static liquibase.util.StringUtil.join; - /** * AbstractJdbcDatabase is extended by all supported databases as a facade to the underlying database. @@ -1224,20 +1222,15 @@ public int hashCode() { @Override public void close() throws DatabaseException { - Scope.getCurrentScope().getSingleton(ExecutorService.class).clearExecutor("jdbc", this); - DatabaseConnection connection = getConnection(); - if (connection != null) { - if (previousAutoCommit != null) { - try { - connection.setAutoCommit(previousAutoCommit); - } catch (DatabaseException e) { - Scope.getCurrentScope().getLog(getClass()).warning("Failed to restore the auto commit to " + previousAutoCommit); - - throw e; - } - } - connection.close(); - } + Scope.getCurrentScope().getSingleton(ExecutorService.class).clearExecutor("jdbc", this); + try (final DatabaseConnection connection = getConnection();) { + if (connection != null && previousAutoCommit != null) { + connection.setAutoCommit(previousAutoCommit); + } + } catch (final DatabaseException e) { + Scope.getCurrentScope().getLog(getClass()).warning("Failed to restore the auto commit to " + previousAutoCommit); + throw e; + } } @Override diff --git a/liquibase-core/src/main/java/liquibase/database/Database.java b/liquibase-core/src/main/java/liquibase/database/Database.java index d59b811184b..1411b306919 100644 --- a/liquibase-core/src/main/java/liquibase/database/Database.java +++ b/liquibase-core/src/main/java/liquibase/database/Database.java @@ -1,25 +1,28 @@ package liquibase.database; +import java.io.IOException; +import java.io.Writer; +import java.math.BigInteger; +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Locale; import liquibase.CatalogAndSchema; import liquibase.change.Change; import liquibase.changelog.ChangeSet; import liquibase.changelog.DatabaseChangeLog; import liquibase.changelog.RanChangeSet; -import liquibase.exception.*; +import liquibase.exception.DatabaseException; +import liquibase.exception.DatabaseHistoryException; +import liquibase.exception.DateParseException; +import liquibase.exception.LiquibaseException; +import liquibase.exception.ValidationErrors; import liquibase.servicelocator.PrioritizedService; import liquibase.sql.visitor.SqlVisitor; import liquibase.statement.DatabaseFunction; import liquibase.statement.SqlStatement; import liquibase.structure.DatabaseObject; -import java.io.IOException; -import java.io.Writer; -import java.math.BigInteger; -import java.util.Collection; -import java.util.Date; -import java.util.List; -import java.util.Locale; - /** * Interface that every DBMS supported by this software must implement. Most methods belong into ont of these * categories: @@ -29,7 +32,7 @@ *
  • creating strings for use in SQL statements, e.g. literals for dates, time, numerals, etc.
    • * */ -public interface Database extends PrioritizedService { +public interface Database extends PrioritizedService, AutoCloseable { String databaseChangeLogTableName = "DatabaseChangeLog".toUpperCase(Locale.US); String databaseChangeLogLockTableName = "DatabaseChangeLogLock".toUpperCase(Locale.US); @@ -290,6 +293,7 @@ public interface Database extends PrioritizedService { String escapeStringForDatabase(String string); + @Override void close() throws DatabaseException; boolean supportsRestrictForeignKeys(); diff --git a/liquibase-core/src/main/java/liquibase/database/DatabaseConnection.java b/liquibase-core/src/main/java/liquibase/database/DatabaseConnection.java index b1da23fc667..98ce7098267 100644 --- a/liquibase-core/src/main/java/liquibase/database/DatabaseConnection.java +++ b/liquibase-core/src/main/java/liquibase/database/DatabaseConnection.java @@ -1,10 +1,9 @@ package liquibase.database; -import liquibase.exception.DatabaseException; -import liquibase.servicelocator.PrioritizedService; - import java.sql.Driver; import java.util.Properties; +import liquibase.exception.DatabaseException; +import liquibase.servicelocator.PrioritizedService; /** * A liquibase abstraction over the normal Connection that is available in @@ -12,7 +11,7 @@ * connection. * */ -public interface DatabaseConnection extends PrioritizedService { +public interface DatabaseConnection extends PrioritizedService, AutoCloseable { void open(String url, Driver driverObject, Properties driverProperties) throws DatabaseException; @@ -29,6 +28,7 @@ default boolean supports(String url) { return true; } + @Override void close() throws DatabaseException; void commit() throws DatabaseException; diff --git a/liquibase-core/src/main/java/liquibase/database/OfflineConnection.java b/liquibase-core/src/main/java/liquibase/database/OfflineConnection.java index eb498390d64..442b473b3af 100644 --- a/liquibase-core/src/main/java/liquibase/database/OfflineConnection.java +++ b/liquibase-core/src/main/java/liquibase/database/OfflineConnection.java @@ -11,7 +11,6 @@ import liquibase.parser.SnapshotParserFactory; import liquibase.resource.ResourceAccessor; import liquibase.servicelocator.LiquibaseService; -import liquibase.servicelocator.PrioritizedService; import liquibase.snapshot.DatabaseSnapshot; import liquibase.snapshot.EmptyDatabaseSnapshot; import liquibase.snapshot.InvalidExampleException; diff --git a/liquibase-core/src/main/java/liquibase/database/core/DerbyDatabase.java b/liquibase-core/src/main/java/liquibase/database/core/DerbyDatabase.java index a731d5587fd..0cd79a5f946 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/DerbyDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/core/DerbyDatabase.java @@ -59,7 +59,7 @@ public String getDefaultDriver(String url) { } catch (ClassNotFoundException classNotFoundException) { // Return class for newer versions anyway return derbyNewDriverClassName; - } + } } } else if (url.startsWith("jdbc:derby") || url.startsWith("java:derby")) { //Use EmbeddedDriver if using a derby URL but without the `://` in it @@ -153,12 +153,15 @@ public String getViewDefinition(CatalogAndSchema schema, String name) throws Dat @Override public void close() throws DatabaseException { + // FIXME Seems not to be a good way to handle the possibility of getting `getConnection() == null` + if (getConnection() != null) { String url = getConnection().getURL(); String driverName = getDefaultDriver(url); super.close(); - if (getShutdownEmbeddedDerby() && (driverName != null) && driverName.toLowerCase().contains("embedded")) { + if (shutdownEmbeddedDerby && (driverName != null) && driverName.toLowerCase().contains("embedded")) { shutdownDerby(url, driverName); } + } } protected void shutdownDerby(String url, String driverName) throws DatabaseException { @@ -194,7 +197,6 @@ protected void shutdownDerby(String url, String driverName) throws DatabaseExcep /** * Determine Apache Derby driver major/minor version. */ - @SuppressWarnings({"static-access", "unchecked"}) protected void determineDriverVersion() { try { // Locate the Derby sysinfo class and query its version info diff --git a/liquibase-core/src/test/java/liquibase/database/AbstractJdbcDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/AbstractJdbcDatabaseTest.java index da4f96d13b7..bb9c5e1fb6e 100644 --- a/liquibase-core/src/test/java/liquibase/database/AbstractJdbcDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/AbstractJdbcDatabaseTest.java @@ -1,7 +1,14 @@ package liquibase.database; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import java.util.ArrayList; +import java.util.List; +import org.junit.Test; import liquibase.Scope; import liquibase.change.core.CreateTableChange; +import liquibase.exception.DatabaseException; import liquibase.executor.ExecutorService; import liquibase.sdk.executor.MockExecutor; import liquibase.sql.visitor.AppendSqlVisitor; @@ -9,14 +16,6 @@ import liquibase.statement.SqlStatement; import liquibase.statement.core.DropTableStatement; import liquibase.structure.core.Table; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.List; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertFalse; /** * Base test class for database-specific tests @@ -88,14 +87,14 @@ public void defaultsWorkWithoutAConnection() { // } @Test - public void escapeTableName_noSchema() { - Database database = getDatabase(); + public void escapeTableName_noSchema() throws DatabaseException { + final Database database = getDatabase(); assertEquals("tableName", database.escapeTableName(null, null, "tableName")); } @Test - public void escapeTableName_withSchema() { - Database database = getDatabase(); + public void escapeTableName_withSchema() throws DatabaseException { + final Database database = getDatabase(); if (database.supportsCatalogInObjectName(Table.class)) { assertEquals("catalogName.schemaName.tableName", database.escapeTableName("catalogName", "schemaName", "tableName")); } else { @@ -283,7 +282,7 @@ public void test_escapeObjectName() { assertTrue(tableName.matches("[\\[\\\"`]?My Table [\\]\\\"`]?")); tableName = database.escapeObjectName("MyTable", Table.class); - assertTrue(tableName.equals("MyTable")); + assertEquals("MyTable", tableName); tableName = database.escapeObjectName("My Table", Table.class); assertTrue(tableName.matches("[\\[\\\"`]?My Table[\\]\\\"`]?")); diff --git a/liquibase-core/src/test/java/liquibase/database/ConnectionServiceFactoryTest.java b/liquibase-core/src/test/java/liquibase/database/ConnectionServiceFactoryTest.java index efe63fa3167..0d5ac9e2490 100644 --- a/liquibase-core/src/test/java/liquibase/database/ConnectionServiceFactoryTest.java +++ b/liquibase-core/src/test/java/liquibase/database/ConnectionServiceFactoryTest.java @@ -1,13 +1,9 @@ package liquibase.database; -import liquibase.database.jvm.JdbcConnection; +import static org.assertj.core.api.Assertions.assertThat; import org.junit.Before; import org.junit.Test; - -import java.sql.Driver; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; +import liquibase.database.jvm.JdbcConnection; public class ConnectionServiceFactoryTest { diff --git a/liquibase-core/src/test/java/liquibase/database/core/AbstractDb2DatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/AbstractDb2DatabaseTest.java index db99a27a6c8..7690ebb352f 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/AbstractDb2DatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/AbstractDb2DatabaseTest.java @@ -1,15 +1,19 @@ package liquibase.database.core; import junit.framework.TestCase; -import liquibase.database.Database; +import liquibase.exception.DatabaseException; public class AbstractDb2DatabaseTest extends TestCase { - public void testGetDateLiteral() { - AbstractDb2Database database = new DB2Database(); - assertEquals("DATE('2018-12-31')", database.getDateLiteral("2018-12-31")); - assertEquals("TIME('23:58:59')", database.getDateLiteral("23:58:59")); - assertEquals("TIMESTAMP('2018-12-31 23:58:59')", database.getDateLiteral("2018-12-31 23:58:59")); - assertEquals("UNSUPPORTED:foo", database.getDateLiteral("foo")); + public void testGetDateLiteral() throws DatabaseException { + try (AbstractDb2Database database = new DB2Database()) { + assertEquals("DATE('2018-12-31')", database.getDateLiteral("2018-12-31")); + assertEquals("TIME('23:58:59')", database.getDateLiteral("23:58:59")); + assertEquals("TIMESTAMP('2018-12-31 23:58:59')", database.getDateLiteral("2018-12-31 23:58:59")); + assertEquals("UNSUPPORTED:foo", database.getDateLiteral("foo")); + } catch (final DatabaseException e) { + throw e; + } } + } diff --git a/liquibase-core/src/test/java/liquibase/database/core/DB2DatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/DB2DatabaseTest.java index 33a017efd7c..9546275e1a4 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/DB2DatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/DB2DatabaseTest.java @@ -2,15 +2,18 @@ import junit.framework.TestCase; import liquibase.database.Database; +import liquibase.exception.DatabaseException; public class DB2DatabaseTest extends TestCase { - public void testGetDefaultDriver() { - Database database = new DB2Database(); - assertEquals("com.ibm.db2.jcc.DB2Driver", database.getDefaultDriver("jdbc:db2://localhost:50000/liquibas")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new DB2Database()) { + assertEquals("com.ibm.db2.jcc.DB2Driver", database.getDefaultDriver("jdbc:db2://localhost:50000/liquibas")); - assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } - } diff --git a/liquibase-core/src/test/java/liquibase/database/core/DB2zDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/DB2zDatabaseTest.java index 95750b66846..9b3533f538d 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/DB2zDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/DB2zDatabaseTest.java @@ -2,15 +2,18 @@ import junit.framework.TestCase; import liquibase.database.Database; +import liquibase.exception.DatabaseException; public class DB2zDatabaseTest extends TestCase { - public void testGetDefaultDriver() { - Database database = new Db2zDatabase(); - assertEquals("com.ibm.db2.jcc.DB2Driver", database.getDefaultDriver("jdbc:db2://localhost:50000/liquibas")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new Db2zDatabase()) { + assertEquals("com.ibm.db2.jcc.DB2Driver", database.getDefaultDriver("jdbc:db2://localhost:50000/liquibas")); - assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } - } diff --git a/liquibase-core/src/test/java/liquibase/database/core/DerbyDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/DerbyDatabaseTest.java index 5b2032eec16..68eb67178d5 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/DerbyDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/DerbyDatabaseTest.java @@ -1,7 +1,7 @@ package liquibase.database.core; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.RETURNS_SMART_NULLS; -import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -19,12 +19,15 @@ import liquibase.exception.DatabaseException; public class DerbyDatabaseTest extends TestCase { - public void testGetDefaultDriver() { - Database database = new DerbyDatabase(); - assertEquals("org.apache.derby.jdbc.EmbeddedDriver", database.getDefaultDriver("java:derby:liquibase;create=true")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new DerbyDatabase()) { + assertEquals("org.apache.derby.jdbc.EmbeddedDriver", database.getDefaultDriver("java:derby:liquibase;create=true")); - assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } public void testGetDateLiteral() { diff --git a/liquibase-core/src/test/java/liquibase/database/core/MSSQLDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/MSSQLDatabaseTest.java index fd965be4d15..fffb58f4121 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/MSSQLDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/MSSQLDatabaseTest.java @@ -1,10 +1,15 @@ package liquibase.database.core; -import liquibase.database.*; -import liquibase.test.JUnitResourceAccessor; -import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; -import static org.junit.Assert.*; +import org.junit.Test; +import liquibase.database.AbstractJdbcDatabaseTest; +import liquibase.database.Database; +import liquibase.database.DatabaseFactory; +import liquibase.exception.DatabaseException; /** * Tests for {@link MSSQLDatabase} @@ -20,7 +25,6 @@ protected String getProductNameString() { return "Microsoft SQL Server"; } - @Override @Test public void supportsInitiallyDeferrableColumns() { @@ -34,31 +38,38 @@ public void getCurrentDateTimeFunction() { } @Test - public void getDefaultDriver() { - Database database = new MSSQLDatabase(); - - assertEquals("com.microsoft.sqlserver.jdbc.SQLServerDriver", database.getDefaultDriver("jdbc:sqlserver://localhost;databaseName=liquibase")); - - assertNull(database.getDefaultDriver("jdbc:oracle:thin://localhost;databaseName=liquibase")); + public void getDefaultDriver() throws DatabaseException { + try (Database database = new MSSQLDatabase()) { + assertEquals("com.microsoft.sqlserver.jdbc.SQLServerDriver", database.getDefaultDriver("jdbc:sqlserver://localhost;databaseName=liquibase")); + + assertNull(database.getDefaultDriver("jdbc:oracle:thin://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } @Override @Test - public void escapeTableName_noSchema() { - Database database = new MSSQLDatabase(); - assertEquals("tableName", database.escapeTableName(null, null, "tableName")); - assertEquals("[tableName€]", database.escapeTableName(null, null, "tableName€")); + public void escapeTableName_noSchema() throws DatabaseException { + try (Database database = new MSSQLDatabase()) { + assertEquals("tableName", database.escapeTableName(null, null, "tableName")); + assertEquals("[tableName€]", database.escapeTableName(null, null, "tableName€")); + } catch (final DatabaseException e) { + throw e; + } } @Override @Test - public void escapeTableName_withSchema() { - Database database = new MSSQLDatabase(); - assertEquals("catalogName.schemaName.tableName", database.escapeTableName("catalogName", "schemaName", - "tableName")); - assertEquals("[catalogName€].[schemaName€].[tableName€]", database.escapeTableName("catalogName€", - "schemaName€", "tableName€")); + public void escapeTableName_withSchema() throws DatabaseException { + try (Database database = new MSSQLDatabase()) { + assertEquals("catalogName.schemaName.tableName", database.escapeTableName("catalogName", "schemaName", "tableName")); + assertEquals("[catalogName€].[schemaName€].[tableName€]", database.escapeTableName("catalogName€", "schemaName€", "tableName€")); + } catch (final DatabaseException e) { + throw e; + } } + private Database createOfflineDatabase(String url) throws Exception { return DatabaseFactory.getInstance().openDatabase(url, null, null, null, null); } @@ -68,15 +79,15 @@ public void setDefaultSchemaName() throws Exception { // // No exception should be thrown by call to setDefaultSchemaName // - Database database = createOfflineDatabase("offline:mssql"); + final Database database = createOfflineDatabase("offline:mssql"); database.setDefaultSchemaName("MySchema"); } @Test public void isUnmodifiable() throws Exception { - Database database = createOfflineDatabase("offline:mssql"); + final Database database = createOfflineDatabase("offline:mssql"); assertTrue(database instanceof MSSQLDatabase); - MSSQLDatabase mssqlDatabase = (MSSQLDatabase)database; + final MSSQLDatabase mssqlDatabase = (MSSQLDatabase) database; assertTrue(mssqlDatabase.dataTypeIsNotModifiable("datetime")); } diff --git a/liquibase-core/src/test/java/liquibase/database/core/MySQLDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/MySQLDatabaseTest.java index dc0f0c411d4..6b0aa06595a 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/MySQLDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/MySQLDatabaseTest.java @@ -1,13 +1,15 @@ package liquibase.database.core; -import liquibase.database.AbstractJdbcDatabase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; + +import org.junit.Assert; +import org.junit.Test; import liquibase.database.AbstractJdbcDatabaseTest; import liquibase.database.Database; +import liquibase.exception.DatabaseException; import liquibase.statement.DatabaseFunction; -import org.junit.Assert; -import org.junit.Test; - -import static org.junit.Assert.*; /** * Tests for {@link MySQLDatabase} @@ -20,7 +22,7 @@ public MySQLDatabaseTest() throws Exception { @Override protected String getProductNameString() { - return "MySQL"; + return "MySQL"; } @Override @@ -29,8 +31,6 @@ public void supportsInitiallyDeferrableColumns() { assertFalse(getDatabase().supportsInitiallyDeferrableColumns()); } - - @Override @Test public void getCurrentDateTimeFunction() { @@ -39,7 +39,7 @@ public void getCurrentDateTimeFunction() { @Test public void getCurrentDateTimeFunctionWithPrecision() { - MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); + final MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); Assert.assertEquals("NOW(1)", mySQLDatabase.getCurrentDateTimeFunction(1)); Assert.assertEquals("NOW(2)", mySQLDatabase.getCurrentDateTimeFunction(2)); Assert.assertEquals("NOW(5)", mySQLDatabase.getCurrentDateTimeFunction(5)); @@ -47,43 +47,45 @@ public void getCurrentDateTimeFunctionWithPrecision() { @Test public void generateDatabaseFunctionValue() { - MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); + final MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); assertEquals("NOW()", mySQLDatabase.generateDatabaseFunctionValue(new DatabaseFunction("CURRENT_TIMESTAMP()"))); assertNull(mySQLDatabase.generateDatabaseFunctionValue(new DatabaseFunction(null))); } @Test public void generateDatabaseFunctionValueWithPrecision() { - MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); + final MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); assertEquals("NOW(2)", mySQLDatabase.generateDatabaseFunctionValue(new DatabaseFunction("CURRENT_TIMESTAMP(2)"))); assertEquals("NOW(3)", mySQLDatabase.generateDatabaseFunctionValue(new DatabaseFunction("CURRENT_TIMESTAMP(3)"))); } @Test public void generateDatabaseFunctionValueWithIncorrectPrecision() { - MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); + final MySQLDatabase mySQLDatabase = (MySQLDatabase) getDatabase(); assertEquals("NOW()", mySQLDatabase.generateDatabaseFunctionValue(new DatabaseFunction("CURRENT_TIMESTAMP(string)"))); } - public void testGetDefaultDriver() { - Database database = new MySQLDatabase(); - - assertEquals("com.mysql.cj.jdbc.Driver", database.getDefaultDriver("jdbc:mysql://localhost/liquibase")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new MySQLDatabase()) { + assertEquals("com.mysql.cj.jdbc.Driver", database.getDefaultDriver("jdbc:mysql://localhost/liquibase")); - assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } @Override @Test public void escapeTableName_noSchema() { - Database database = getDatabase(); + final Database database = getDatabase(); assertEquals("tableName", database.escapeTableName(null, null, "tableName")); } @Override @Test public void escapeTableName_withSchema() { - Database database = getDatabase(); + final Database database = getDatabase(); assertEquals("catalogName.tableName", database.escapeTableName("catalogName", "schemaName", "tableName")); } diff --git a/liquibase-core/src/test/java/liquibase/database/core/OracleDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/OracleDatabaseTest.java index f75d10efb5d..a86dff84874 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/OracleDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/OracleDatabaseTest.java @@ -1,5 +1,17 @@ package liquibase.database.core; +import static java.util.ResourceBundle.getBundle; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.ResourceBundle; + +import org.hamcrest.CoreMatchers; +import org.junit.Assert; +import org.junit.Test; import liquibase.Scope; import liquibase.database.AbstractJdbcDatabaseTest; import liquibase.database.Database; @@ -7,6 +19,7 @@ import liquibase.database.OfflineConnection; import liquibase.datatype.DatabaseDataType; import liquibase.datatype.core.TimestampType; +import liquibase.exception.DatabaseException; import liquibase.exception.LiquibaseException; import liquibase.executor.ExecutorService; import liquibase.resource.ResourceAccessor; @@ -16,18 +29,6 @@ import liquibase.statement.SqlStatement; import liquibase.statement.core.UpdateStatement; import liquibase.test.JUnitResourceAccessor; -import org.hamcrest.CoreMatchers; -import org.junit.Assert; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.ResourceBundle; - -import static java.util.ResourceBundle.getBundle; -import static org.junit.Assert.*; - -import org.hamcrest.CoreMatchers; -import org.junit.Test; /** * Tests for {@link liquibase.database.core.OracleDatabase}. @@ -48,72 +49,67 @@ protected String getProductNameString() { @Override @Test public void escapeTableName_noSchema() { - Database database = getDatabase(); - assertEquals("table name without schema is correctly escaped as simply tableName", - "tableName", database.escapeTableName(null, null, "tableName")); + final Database database = getDatabase(); + assertEquals("table name without schema is correctly escaped as simply tableName", "tableName", database.escapeTableName(null, null, "tableName")); } @Test public void saveNlsEnvironment() throws Exception { - Database database = getDatabase(); - ResourceAccessor junitResourceAccessor = new JUnitResourceAccessor(); - OfflineConnection offlineConnection = new OfflineConnection("offline:oracle", junitResourceAccessor); + final Database database = getDatabase(); + final ResourceAccessor junitResourceAccessor = new JUnitResourceAccessor(); + final OfflineConnection offlineConnection = new OfflineConnection("offline:oracle", junitResourceAccessor); database.setConnection(offlineConnection); } @Override @Test public void escapeTableName_withSchema() { - Database database = getDatabase(); - assertEquals("table name without schema but with catalog is correctly escaped as catalogName.tableName", - "catalogName.tableName", database.escapeTableName("catalogName", "schemaName", "tableName")); + final Database database = getDatabase(); + assertEquals("table name without schema but with catalog is correctly escaped as catalogName.tableName", "catalogName.tableName", database.escapeTableName("catalogName", "schemaName", "tableName")); } @Override @Test public void supportsInitiallyDeferrableColumns() { - assertTrue("Oracle Database is correctly reported as being able to do INITIALLY DEFERRED column constraints.", - getDatabase().supportsInitiallyDeferrableColumns()); + assertTrue("Oracle Database is correctly reported as being able to do INITIALLY DEFERRED column constraints.", getDatabase().supportsInitiallyDeferrableColumns()); } - @Override @Test public void getCurrentDateTimeFunction() { - Assert.assertEquals("Oracle Database's 'give me the current timestamp' function is correctly reported.", - "SYSTIMESTAMP", getDatabase().getCurrentDateTimeFunction()); + Assert.assertEquals("Oracle Database's 'give me the current timestamp' function is correctly reported.", "SYSTIMESTAMP", getDatabase().getCurrentDateTimeFunction()); } @Test public void verifyTimestampDataTypeWhenWithoutClauseIsPresent() { - TimestampType ts = new TimestampType(); + final TimestampType ts = new TimestampType(); ts.setAdditionalInformation("WITHOUT TIME ZONE"); - DatabaseDataType oracleDataType = ts.toDatabaseDataType(getDatabase()); + final DatabaseDataType oracleDataType = ts.toDatabaseDataType(getDatabase()); assertThat(oracleDataType.getType(), CoreMatchers.is("TIMESTAMP")); } - public void testGetDefaultDriver() { - Database database = new OracleDatabase(); - - assertEquals("The correct JDBC driver class name is reported if the URL is a Oracle JDBC URL", - "oracle.jdbc.OracleDriver", database.getDefaultDriver("jdbc:oracle:thin:@localhost/XE")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new OracleDatabase()) { + assertEquals("The correct JDBC driver class name is reported if the URL is a Oracle JDBC URL", "oracle.jdbc.OracleDriver", database.getDefaultDriver("jdbc:oracle:thin:@localhost/XE")); - assertNull("No JDBC driver class is returned if the URL is NOT an Oracle Database JDBC URL.", - database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + assertNull("No JDBC driver class is returned if the URL is NOT an Oracle Database JDBC URL.", database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } @Test public void validateCore2953WrongSqlOnValueSequenceNext() throws LiquibaseException { - Database database = getDatabase(); + final Database database = getDatabase(); database.setObjectQuotingStrategy(ObjectQuotingStrategy.QUOTE_ALL_OBJECTS); database.setDefaultSchemaName("sampleschema"); - MockExecutor mockExecutor = new MockExecutor(); + final MockExecutor mockExecutor = new MockExecutor(); mockExecutor.setDatabase(database); Scope.getCurrentScope().getSingleton(ExecutorService.class).setExecutor("jdbc", database, mockExecutor); - UpdateStatement updateStatement = new UpdateStatement(null, null, "test_table"); + final UpdateStatement updateStatement = new UpdateStatement(null, null, "test_table"); updateStatement.addNewColumnValue("id", new SequenceNextValueFunction("test_table_id_seq")); database.execute(new SqlStatement[]{updateStatement}, new ArrayList()); diff --git a/liquibase-core/src/test/java/liquibase/database/core/PostgresDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/PostgresDatabaseTest.java index 87bf907ddc8..7ed5ed54439 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/PostgresDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/PostgresDatabaseTest.java @@ -1,16 +1,19 @@ package liquibase.database.core; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Assert; +import org.junit.Test; import liquibase.GlobalConfiguration; import liquibase.changelog.column.LiquibaseColumn; import liquibase.database.AbstractJdbcDatabaseTest; import liquibase.database.Database; import liquibase.database.ObjectQuotingStrategy; +import liquibase.exception.DatabaseException; import liquibase.structure.core.Table; import liquibase.util.StringUtil; -import org.junit.Assert; -import org.junit.Test; - -import static org.junit.Assert.*; /** * Tests for {@link PostgresDatabase} @@ -49,12 +52,14 @@ public void testCheckDatabaseChangeLogTable() throws Exception { ; //TODO: test has troubles, fix later } - public void testGetDefaultDriver() { - Database database = new PostgresDatabase(); - - assertEquals("org.postgresql.Driver", database.getDefaultDriver("jdbc:postgresql://localhost/liquibase")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new PostgresDatabase()) { + assertEquals("org.postgresql.Driver", database.getDefaultDriver("jdbc:postgresql://localhost/liquibase")); - assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } diff --git a/liquibase-core/src/test/java/liquibase/database/core/UnsupportedDatabaseTest.java b/liquibase-core/src/test/java/liquibase/database/core/UnsupportedDatabaseTest.java index a1c23c9e4f3..eb0bb10ae39 100644 --- a/liquibase-core/src/test/java/liquibase/database/core/UnsupportedDatabaseTest.java +++ b/liquibase-core/src/test/java/liquibase/database/core/UnsupportedDatabaseTest.java @@ -2,17 +2,21 @@ import junit.framework.TestCase; import liquibase.database.Database; +import liquibase.exception.DatabaseException; public class UnsupportedDatabaseTest extends TestCase { - public void testGetDefaultDriver() { - Database database = new UnsupportedDatabase(); - assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); - assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); - assertNull(database.getDefaultDriver("jdbc:hsqldb://localhost;databaseName=liquibase")); - assertNull(database.getDefaultDriver("jdbc:derby://localhost;databaseName=liquibase")); - assertNull(database.getDefaultDriver("jdbc:sqlserver://localhost;databaseName=liquibase")); - assertNull(database.getDefaultDriver("jdbc:postgresql://localhost;databaseName=liquibase")); + public void testGetDefaultDriver() throws DatabaseException { + try (Database database = new UnsupportedDatabase()) { + assertNull(database.getDefaultDriver("jdbc:oracle://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:db2://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:hsqldb://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:derby://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:sqlserver://localhost;databaseName=liquibase")); + assertNull(database.getDefaultDriver("jdbc:postgresql://localhost;databaseName=liquibase")); + } catch (final DatabaseException e) { + throw e; + } } } From 8cff0ce056a3580e7bce32ccefc4b36a77f9f148 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 20 Jul 2022 07:44:32 -0700 Subject: [PATCH 11/12] Bump jaxb-runtime from 2.3.6 to 4.0.0 (#2964) Bumps jaxb-runtime from 2.3.6 to 4.0.0. --- updated-dependencies: - dependency-name: org.glassfish.jaxb:jaxb-runtime dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Nathan Voxland --- liquibase-dist/pom.xml | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/liquibase-dist/pom.xml b/liquibase-dist/pom.xml index a79cce9549b..14d0c3ec47c 100644 --- a/liquibase-dist/pom.xml +++ b/liquibase-dist/pom.xml @@ -140,7 +140,7 @@ org.glassfish.jaxb jaxb-runtime - 2.3.6 + 4.0.0 true diff --git a/pom.xml b/pom.xml index 8a96b8ed8cf..4cfd28a639a 100644 --- a/pom.xml +++ b/pom.xml @@ -247,7 +247,7 @@ org.glassfish.jaxb jaxb-runtime - 2.3.6 + 4.0.0 test From eb625608d95e35364fc4c4897f4b8c4cd2a9b542 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Wed, 20 Jul 2022 08:08:22 -0700 Subject: [PATCH 12/12] Added liquibase.changelogParseMode setting (#3057) Added LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER setting for LiquibaseLauncher to better support running in IDEs Added new liquibase.changelogParseMode setting for unknown change and precondition types --- .../commandline/LiquibaseLauncher.java | 24 +++++++-- .../java/liquibase/changelog/ChangeSet.java | 53 ++++++++++--------- .../changelog/DatabaseChangeLog.java | 8 +-- .../parser/ChangeLogParserConfiguration.java | 16 ++++++ .../precondition/PreconditionLogic.java | 6 +++ .../liquibase/changelog/ChangeSetTest.groovy | 36 +++++++++++++ .../YamlChangeLogParser_RealFile_Test.groovy | 23 -------- .../core/PreconditionContainerTest.groovy | 41 ++++++++++++-- .../liquibase/registered-changelog.json | 52 ------------------ .../liquibase/registered-changelog.yml | 28 ---------- .../src/test/resources/liquibase/simple.json | 52 ------------------ .../resources/liquibase/test-changelog.json | 52 ------------------ .../resources/liquibase/test-changelog.yml | 28 ---------- 13 files changed, 145 insertions(+), 274 deletions(-) diff --git a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java index ac56d997914..a92eb15fe82 100644 --- a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java +++ b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java @@ -24,6 +24,12 @@ public static void main(final String[] args) throws Exception { debug("Debug mode enabled because LIQUIBASE_LAUNCHER_DEBUG is set to " + debugSetting); } + String parentLoaderSetting = System.getenv("LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER"); + if (parentLoaderSetting == null) { + parentLoaderSetting = "system"; + } + debug("LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER is set to " + parentLoaderSetting); + final String liquibaseHomeEnv = System.getenv("LIQUIBASE_HOME"); debug("LIQUIBASE_HOME: " + liquibaseHomeEnv); if (liquibaseHomeEnv == null || liquibaseHomeEnv.equals("")) { @@ -80,10 +86,20 @@ public static void main(final String[] args) throws Exception { } } - //loading with the regular system classloader includes liquibase.jar in the parent. - //That causes the parent classloader to load LiqiuabaseCommandLine which makes it not able to access files in the child classloader - //The system classloader's parent is the boot classloader, which keeps the only classloader with liquibase.jar the same as the rest of the classes it needs to access. - final URLClassLoader classloader = new URLClassLoader(urls.toArray(new URL[0]), ClassLoader.getSystemClassLoader().getParent()); + ClassLoader parentLoader; + if (parentLoaderSetting.equalsIgnoreCase("system")) { + //loading with the regular system classloader includes liquibase.jar in the parent. + //That causes the parent classloader to load LiquibaseCommandLine which makes it not able to access files in the child classloader + //The system classloader's parent is the boot classloader, which keeps the only classloader with liquibase.jar the same as the rest of the classes it needs to access. + parentLoader = ClassLoader.getSystemClassLoader().getParent(); + + } else if (parentLoaderSetting.equalsIgnoreCase("thread")) { + parentLoader = Thread.currentThread().getContextClassLoader(); + } else { + throw new RuntimeException("Unknown LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER value: "+parentLoaderSetting); + } + + final URLClassLoader classloader = new URLClassLoader(urls.toArray(new URL[0]), parentLoader); Thread.currentThread().setContextClassLoader(classloader); final Class cli = classloader.loadClass(LiquibaseCommandLine.class.getName()); diff --git a/liquibase-core/src/main/java/liquibase/changelog/ChangeSet.java b/liquibase-core/src/main/java/liquibase/changelog/ChangeSet.java index d37f406dde9..c62849f8845 100644 --- a/liquibase-core/src/main/java/liquibase/changelog/ChangeSet.java +++ b/liquibase-core/src/main/java/liquibase/changelog/ChangeSet.java @@ -1,6 +1,7 @@ package liquibase.changelog; import liquibase.ContextExpression; +import liquibase.GlobalConfiguration; import liquibase.Labels; import liquibase.Scope; import liquibase.change.*; @@ -16,6 +17,7 @@ import liquibase.executor.ExecutorService; import liquibase.executor.LoggingExecutor; import liquibase.logging.Logger; +import liquibase.parser.ChangeLogParserConfiguration; import liquibase.parser.core.ParsedNode; import liquibase.parser.core.ParsedNodeException; import liquibase.precondition.Conditional; @@ -194,7 +196,7 @@ public String toString() { */ private PreconditionContainer preconditions; - /** + /** * ChangeSet level attribute to specify an Executor */ private String runWith; @@ -276,6 +278,7 @@ public String getFilePath() { /** * The logical file path defined directly on this node. Return null if not set. + * * @return */ public String getLogicalFilePath() { @@ -367,7 +370,7 @@ public void load(ParsedNode node, ResourceAccessor resourceAccessor) throws Pars } } else { filePath = filePath.replaceAll("\\\\", "/") - .replaceFirst("^/", ""); + .replaceFirst("^/", ""); } @@ -440,11 +443,7 @@ protected void handleChildNode(ParsedNode child, ResourceAccessor resourceAccess break; case "preConditions": this.preconditions = new PreconditionContainer(); - try { - this.preconditions.load(child, resourceAccessor); - } catch (ParsedNodeException e) { - e.printStackTrace(); - } + this.preconditions.load(child, resourceAccessor); break; case "changes": for (ParsedNode changeNode : child.getChildren()) { @@ -517,6 +516,14 @@ protected void handleRollbackNode(ParsedNode rollbackNode, ResourceAccessor reso protected Change toChange(ParsedNode value, ResourceAccessor resourceAccessor) throws ParsedNodeException { Change change = Scope.getCurrentScope().getSingleton(ChangeFactory.class).create(value.getName()); if (change == null) { + if (value.getChildren().size() > 0 && ChangeLogParserConfiguration.CHANGELOG_PARSE_MODE.getCurrentValue().equals(ChangeLogParserConfiguration.ChangelogParseMode.STRICT)) { + String message = ""; + if (this.getChangeLog() != null && this.getChangeLog().getPhysicalFilePath() != null) { + message = "Error parsing " + this.getChangeLog().getPhysicalFilePath() + ": "; + } + message += "Unknown change type '" + value.getName() + "'. Check for spelling or capitalization errors and missing extensions such as liquibase-commercial."; + throw new ParsedNodeException(message); + } return null; } else { change.load(value, resourceAccessor); @@ -730,7 +737,7 @@ private Executor setupCustomExecutorIfNecessary(Database database) { Scope.getCurrentScope().getSingleton(ExecutorService.class).setExecutor("jdbc", database, customExecutor); List changes = getChanges(); for (Change change : changes) { - if (! (change instanceof AbstractChange)) { + if (!(change instanceof AbstractChange)) { continue; } final ResourceAccessor resourceAccessor = ((AbstractChange) change).getResourceAccessor(); @@ -743,13 +750,11 @@ private Executor setupCustomExecutorIfNecessary(Database database) { } /** - * * Look for a configuration property that matches liquibase..executor * and if found, return its value as the executor name * - * @param executorName The value from the input changeset runWith attribute - * @return String The mapped value - * + * @param executorName The value from the input changeset runWith attribute + * @return String The mapped value */ public static String lookupExecutor(String executorName) { if (StringUtil.isEmpty(executorName)) { @@ -757,7 +762,7 @@ public static String lookupExecutor(String executorName) { } String key = "liquibase." + executorName.toLowerCase() + ".executor"; String replacementExecutorName = - (String)Scope.getCurrentScope().getSingleton(LiquibaseConfiguration.class).getCurrentConfiguredValue(null, null, key).getValue(); + (String) Scope.getCurrentScope().getSingleton(LiquibaseConfiguration.class).getCurrentConfiguredValue(null, null, key).getValue(); if (replacementExecutorName != null) { Scope.getCurrentScope().getLog(ChangeSet.class).info("Mapped '" + executorName + "' to executor '" + replacementExecutorName + "'"); return replacementExecutorName; @@ -906,7 +911,7 @@ public void setIgnore(boolean ignore) { } public boolean isInheritableIgnore() { - DatabaseChangeLog changeLog = getChangeLog(); + DatabaseChangeLog changeLog = getChangeLog(); if (changeLog == null) { return false; } @@ -945,11 +950,9 @@ public Collection getInheritableLabels() { } /** - * * Build and return a string which contains both the changeset and inherited context * - * @return String - * + * @return String */ public String buildFullContext() { StringBuilder contextExpression = new StringBuilder(); @@ -966,11 +969,9 @@ public String buildFullContext() { } /** - * * Build and return a string which contains both the changeset and inherited labels * - * @return String - * + * @return String */ public String buildFullLabels() { StringBuilder labels = new StringBuilder(); @@ -1219,11 +1220,11 @@ public String getSerializedObjectName() { @Override public Set getSerializableFields() { return new LinkedHashSet<>( - Arrays.asList( - "id", "author", "runAlways", "runOnChange", "failOnError", "context", "labels", "dbms", - "objectQuotingStrategy", "comment", "preconditions", "changes", "rollback", "labels", - "logicalFilePath", "created", "runInTransaction", "runOrder", "ignore" - ) + Arrays.asList( + "id", "author", "runAlways", "runOnChange", "failOnError", "context", "labels", "dbms", + "objectQuotingStrategy", "comment", "preconditions", "changes", "rollback", "labels", + "logicalFilePath", "created", "runInTransaction", "runOrder", "ignore" + ) ); } @@ -1308,7 +1309,7 @@ public Object getSerializableFieldValue(String field) { } if ("logicalFilePath".equals(field)) { - return getLogicalFilePath(); + return getLogicalFilePath(); } if ("rollback".equals(field)) { diff --git a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java index d51e3512483..c21e846ba82 100644 --- a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java +++ b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java @@ -426,13 +426,9 @@ protected void handleChildNode(ParsedNode node, ResourceAccessor resourceAccesso } case "preConditions": { PreconditionContainer parsedContainer = new PreconditionContainer(); - try { - parsedContainer.load(node, resourceAccessor); - this.preconditionContainer.addNestedPrecondition(parsedContainer); + parsedContainer.load(node, resourceAccessor); + this.preconditionContainer.addNestedPrecondition(parsedContainer); - } catch (ParsedNodeException e) { - e.printStackTrace(); - } break; } case "property": { diff --git a/liquibase-core/src/main/java/liquibase/parser/ChangeLogParserConfiguration.java b/liquibase-core/src/main/java/liquibase/parser/ChangeLogParserConfiguration.java index f0a6f64ec4b..36afa37be9f 100644 --- a/liquibase-core/src/main/java/liquibase/parser/ChangeLogParserConfiguration.java +++ b/liquibase-core/src/main/java/liquibase/parser/ChangeLogParserConfiguration.java @@ -1,5 +1,6 @@ package liquibase.parser; +import liquibase.GlobalConfiguration; import liquibase.configuration.AutoloadedConfigurations; import liquibase.configuration.ConfigurationDefinition; @@ -12,6 +13,9 @@ public class ChangeLogParserConfiguration implements AutoloadedConfigurations { public static final ConfigurationDefinition USE_PROCEDURE_SCHEMA; public static final ConfigurationDefinition MISSING_PROPERTY_MODE; + public static final ConfigurationDefinition CHANGELOG_PARSE_MODE; + + static { ConfigurationDefinition.Builder builder = new ConfigurationDefinition.Builder("liquibase"); @@ -30,6 +34,13 @@ public class ChangeLogParserConfiguration implements AutoloadedConfigurations { .setDescription("How to handle changelog property expressions where a value is not set. For example, a string '${address}' when no 'address' property was defined. Values can be: 'preserve' which leaves the string as-is, 'empty' which replaces it with an empty string, or 'error' which stops processing with an error.") .setDefaultValue(MissingPropertyMode.PRESERVE) .build(); + + + CHANGELOG_PARSE_MODE = builder.define("changelogParseMode", ChangelogParseMode.class) + .setDescription("Configures how to handle unknown fields in changelog files. Possible values: STRICT which causes parsing to fail, and LAX which continues with the parsing.") + .setDefaultValue(ChangelogParseMode.STRICT) + .build(); + } public enum MissingPropertyMode { @@ -37,4 +48,9 @@ public enum MissingPropertyMode { EMPTY, ERROR, } + + public enum ChangelogParseMode { + STRICT, + LAX, + } } diff --git a/liquibase-core/src/main/java/liquibase/precondition/PreconditionLogic.java b/liquibase-core/src/main/java/liquibase/precondition/PreconditionLogic.java index 8c1eff6338e..fc0d41fc532 100644 --- a/liquibase-core/src/main/java/liquibase/precondition/PreconditionLogic.java +++ b/liquibase-core/src/main/java/liquibase/precondition/PreconditionLogic.java @@ -1,7 +1,9 @@ package liquibase.precondition; +import liquibase.GlobalConfiguration; import liquibase.database.Database; import liquibase.exception.ValidationErrors; +import liquibase.parser.ChangeLogParserConfiguration; import liquibase.parser.core.ParsedNode; import liquibase.parser.core.ParsedNodeException; import liquibase.resource.ResourceAccessor; @@ -47,6 +49,10 @@ public void load(ParsedNode parsedNode, ResourceAccessor resourceAccessor) throw protected Precondition toPrecondition(ParsedNode node, ResourceAccessor resourceAccessor) throws ParsedNodeException { Precondition precondition = PreconditionFactory.getInstance().create(node.getName()); if (precondition == null) { + if (node.getChildren() != null && node.getChildren().size() > 0 && ChangeLogParserConfiguration.CHANGELOG_PARSE_MODE.getCurrentValue().equals(ChangeLogParserConfiguration.ChangelogParseMode.STRICT)) { + throw new ParsedNodeException("Unknown precondition '" + node.getName() + "'. Check for spelling or capitalization errors and missing extensions such as liquibase-commercial."); + } + return null; } diff --git a/liquibase-core/src/test/groovy/liquibase/changelog/ChangeSetTest.groovy b/liquibase-core/src/test/groovy/liquibase/changelog/ChangeSetTest.groovy index 5c8b3155106..92aac0c246e 100644 --- a/liquibase-core/src/test/groovy/liquibase/changelog/ChangeSetTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/changelog/ChangeSetTest.groovy @@ -1,7 +1,10 @@ package liquibase.changelog + +import liquibase.Scope import liquibase.change.CheckSum import liquibase.change.core.* +import liquibase.parser.ChangeLogParserConfiguration import liquibase.parser.core.ParsedNode import liquibase.parser.core.ParsedNodeException import liquibase.precondition.core.RunningAsPrecondition @@ -187,6 +190,39 @@ public class ChangeSetTest extends Specification { changeSet.changes[1].tableName == "table_2" } + def "load node with unknown change types and strict parsing"() { + when: + def changeSet = new ChangeSet(new DatabaseChangeLog("com/example/test.xml")) + def node = new ParsedNode(null, "changeSet") + .addChildren([id: "1", author: "nvoxland"]) + .addChild(new ParsedNode(null, "createTable").addChild(null, "tableName", "table_1")) + .addChild(new ParsedNode(null, "invalid").addChild(null, "tableName", "table_2")) + changeSet.load(node, resourceSupplier.simpleResourceAccessor) + + then: + def e = thrown(ParsedNodeException) + e.message == "Error parsing com/example/test.xml: Unknown change type 'invalid'. Check for spelling or capitalization errors and missing extensions such as liquibase-commercial." + } + + def "load node with unknown change types and lax parsing"() { + when: + def changeSet = new ChangeSet(new DatabaseChangeLog("com/example/test.xml")) + def node = new ParsedNode(null, "changeSet") + .addChildren([id: "1", author: "nvoxland"]) + .addChild(new ParsedNode(null, "createTable").addChild(null, "tableName", "table_1")) + .addChild(new ParsedNode(null, "invalid").addChild(null, "tableName", "table_2")) + + Scope.child([(ChangeLogParserConfiguration.CHANGELOG_PARSE_MODE.getKey()): ChangeLogParserConfiguration.ChangelogParseMode.LAX], { + -> + changeSet.load(node, resourceSupplier.simpleResourceAccessor) + } as Scope.ScopedRunner) + + + then: + notThrown(ParsedNodeException) + changeSet.getChanges().size() == 1 + } + def "load node with rollback containing sql as value"() { when: def changeSet = new ChangeSet(new DatabaseChangeLog("com/example/test.xml")) diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy index 0dfeed9bc90..f269d61df2f 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy @@ -300,29 +300,6 @@ public class YamlChangeLogParser_RealFile_Test extends Specification { assert e.message.startsWith("Syntax error in file liquibase/parser/core/yaml/malformedChangeLog.yaml") } - def "elements that don't correspond to anything in liquibase are ignored"() throws Exception { - def path = "liquibase/parser/core/yaml/unusedTagsChangeLog.yaml" - expect: - DatabaseChangeLog changeLog = new YamlChangeLogParser().parse(path, new ChangeLogParameters(), new JUnitResourceAccessor()); - - changeLog.getLogicalFilePath() == path - changeLog.getPhysicalFilePath() == path - - changeLog.getPreconditions().getNestedPreconditions().size() == 0 - changeLog.getChangeSets().size() == 1 - - ChangeSet changeSet = changeLog.getChangeSets().get(0); - changeSet.getAuthor() == "nvoxland" - changeSet.getId() == "1" - changeSet.getChanges().size() == 1 - changeSet.getFilePath() == path - changeSet.getComments() == "Some comments go here" - - Change change = changeSet.getChanges().get(0); - Scope.getCurrentScope().getSingleton(ChangeFactory.class).getChangeMetaData(change).getName() == "createTable" - assert change instanceof CreateTableChange - } - def "changeLog parameters are correctly expanded"() throws Exception { when: def params = new ChangeLogParameters(new MockDatabase()); diff --git a/liquibase-core/src/test/groovy/liquibase/precondition/core/PreconditionContainerTest.groovy b/liquibase-core/src/test/groovy/liquibase/precondition/core/PreconditionContainerTest.groovy index 8cf9b1ccce9..2f19abdfc45 100644 --- a/liquibase-core/src/test/groovy/liquibase/precondition/core/PreconditionContainerTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/precondition/core/PreconditionContainerTest.groovy @@ -1,6 +1,9 @@ package liquibase.precondition.core +import liquibase.GlobalConfiguration +import liquibase.Scope import liquibase.exception.SetupException +import liquibase.parser.ChangeLogParserConfiguration import liquibase.parser.core.ParsedNode import liquibase.parser.core.ParsedNodeException import liquibase.sdk.supplier.resource.ResourceSupplier @@ -9,7 +12,8 @@ import spock.lang.Specification class PreconditionContainerTest extends Specification { - @Shared resourceSupplier = new ResourceSupplier() + @Shared + resourceSupplier = new ResourceSupplier() def "load handles empty node with params"() { when: @@ -101,8 +105,8 @@ class PreconditionContainerTest extends Specification { def node = new ParsedNode(null, "preConditions").addChildren([onFail: "MARK_RAN"]) .addChild(new ParsedNode(null, "runningAs").addChildren([username: "my_user"])) .addChild(new ParsedNode(null, "or") - .addChildren([runningAs: [username: "other_user"]]) - .addChildren([runningAs: [username: "yet_other_user"]]) + .addChildren([runningAs: [username: "other_user"]]) + .addChildren([runningAs: [username: "yet_other_user"]]) ) .addChild(new ParsedNode(null, "tableExists").addChildren([tableName: "my_table"])) @@ -128,4 +132,35 @@ class PreconditionContainerTest extends Specification { } + def "load handles node with unknown preconditions in strict mode"() { + when: + def node = new ParsedNode(null, "preConditions").addChildren([onFail: "MARK_RAN"]) + .addChild(new ParsedNode(null, "invalid").addChildren([tableName: "my_table"])) + .addChild(new ParsedNode(null, "tableExists").addChildren([tableName: "my_table"])) + + def container = new PreconditionContainer() + container.load(node, resourceSupplier.simpleResourceAccessor) + + then: + def e = thrown(ParsedNodeException) + e.message == "Unknown precondition 'invalid'. Check for spelling or capitalization errors and missing extensions such as liquibase-commercial." + } + + def "load handles node with unknown preconditions in lax mode"() { + when: + def node = new ParsedNode(null, "preConditions").addChildren([onFail: "MARK_RAN"]) + .addChild(new ParsedNode(null, "invalid").addChildren([tableName: "my_table"])) + .addChild(new ParsedNode(null, "tableExists").addChildren([tableName: "my_table"])) + + def container = new PreconditionContainer() + Scope.currentScope.child([(ChangeLogParserConfiguration.CHANGELOG_PARSE_MODE.key): ChangeLogParserConfiguration.ChangelogParseMode.LAX], { + -> + container.load(node, resourceSupplier.simpleResourceAccessor) + } as Scope.ScopedRunner) + + + then: + notThrown(ParsedNodeException) + container.getNestedPreconditions().size() == 1 + } } diff --git a/liquibase-core/src/test/resources/liquibase/registered-changelog.json b/liquibase-core/src/test/resources/liquibase/registered-changelog.json index ed23e7f31be..ee5c8244d15 100644 --- a/liquibase-core/src/test/resources/liquibase/registered-changelog.json +++ b/liquibase-core/src/test/resources/liquibase/registered-changelog.json @@ -46,24 +46,6 @@ } }, - { - "changeSet": { - "id": "1604958377003-3", - "author": "wesley (generated)", - "changes": [ - { - "createTrigger": { - "disabled": false, - "path": "objects/trigger/TRIGGER1.sql", - "relativeToChangelogFile": true, - "tableName": "ACCOUNT", - "triggerName": "TRIGGER1" - } - }] - - } - }, - { "changeSet": { "id": "1604958377003-4", @@ -413,40 +395,6 @@ } }] - } - }, - - { - "changeSet": { - "id": "1604958377003-14", - "author": "wesley (generated)", - "changes": [ - { - "addCheckConstraint": { - "constraintBody": "supplier_id BETWEEN 100 and 9999", - "constraintName": "CHECK_SUPPLIER_ID", - "disabled": false, - "tableName": "SUPPLIERS" - } - }] - - } - }, - - { - "changeSet": { - "id": "1604958377003-15", - "author": "wesley (generated)", - "changes": [ - { - "createFunction": { - "functionName": "FUNC_COMPUTE_TAX", - "path": "objects/function/FUNC_COMPUTE_TAX.sql", - "relativeToChangelogFile": true - } - }] - } } - ]} diff --git a/liquibase-core/src/test/resources/liquibase/registered-changelog.yml b/liquibase-core/src/test/resources/liquibase/registered-changelog.yml index 52d2ad1da4b..85440377540 100644 --- a/liquibase-core/src/test/resources/liquibase/registered-changelog.yml +++ b/liquibase-core/src/test/resources/liquibase/registered-changelog.yml @@ -22,16 +22,6 @@ databaseChangeLog: path: objects/storedprocedure/PROC_HELLO_WORLD.sql procedureName: PROC_HELLO_WORLD relativeToChangelogFile: true -- changeSet: - id: 1604960286751-3 - author: wesley (generated) - changes: - - createTrigger: - disabled: false - path: objects/trigger/TRIGGER1.sql - relativeToChangelogFile: true - tableName: ACCOUNT - triggerName: TRIGGER1 - changeSet: id: 1604960286751-4 author: wesley (generated) @@ -208,21 +198,3 @@ databaseChangeLog: type: VARCHAR2(50 BYTE) tableName: SUPPLIERS1 tablespace: USERS -- changeSet: - id: 1604960286751-14 - author: wesley (generated) - changes: - - addCheckConstraint: - constraintBody: supplier_id BETWEEN 100 and 9999 - constraintName: CHECK_SUPPLIER_ID - disabled: false - tableName: SUPPLIERS -- changeSet: - id: 1604960286751-15 - author: wesley (generated) - changes: - - createFunction: - functionName: FUNC_COMPUTE_TAX - path: objects/function/FUNC_COMPUTE_TAX.sql - relativeToChangelogFile: true - diff --git a/liquibase-core/src/test/resources/liquibase/simple.json b/liquibase-core/src/test/resources/liquibase/simple.json index 10a53bf8788..49525411a22 100644 --- a/liquibase-core/src/test/resources/liquibase/simple.json +++ b/liquibase-core/src/test/resources/liquibase/simple.json @@ -44,24 +44,6 @@ } }, - { - "changeSet": { - "id": "1604958377003-3", - "author": "wesley (generated)", - "changes": [ - { - "createTrigger": { - "disabled": false, - "path": "objects/trigger/TRIGGER1.sql", - "relativeToChangelogFile": true, - "tableName": "ACCOUNT", - "triggerName": "TRIGGER1" - } - }] - - } - }, - { "changeSet": { "id": "1604958377003-4", @@ -411,40 +393,6 @@ } }] - } - }, - - { - "changeSet": { - "id": "1604958377003-14", - "author": "wesley (generated)", - "changes": [ - { - "addCheckConstraint": { - "constraintBody": "supplier_id BETWEEN 100 and 9999", - "constraintName": "CHECK_SUPPLIER_ID", - "disabled": false, - "tableName": "SUPPLIERS" - } - }] - - } - }, - - { - "changeSet": { - "id": "1604958377003-15", - "author": "wesley (generated)", - "changes": [ - { - "createFunction": { - "functionName": "FUNC_COMPUTE_TAX", - "path": "objects/function/FUNC_COMPUTE_TAX.sql", - "relativeToChangelogFile": true - } - }] - } } - ]} diff --git a/liquibase-core/src/test/resources/liquibase/test-changelog.json b/liquibase-core/src/test/resources/liquibase/test-changelog.json index 10a53bf8788..49525411a22 100644 --- a/liquibase-core/src/test/resources/liquibase/test-changelog.json +++ b/liquibase-core/src/test/resources/liquibase/test-changelog.json @@ -44,24 +44,6 @@ } }, - { - "changeSet": { - "id": "1604958377003-3", - "author": "wesley (generated)", - "changes": [ - { - "createTrigger": { - "disabled": false, - "path": "objects/trigger/TRIGGER1.sql", - "relativeToChangelogFile": true, - "tableName": "ACCOUNT", - "triggerName": "TRIGGER1" - } - }] - - } - }, - { "changeSet": { "id": "1604958377003-4", @@ -411,40 +393,6 @@ } }] - } - }, - - { - "changeSet": { - "id": "1604958377003-14", - "author": "wesley (generated)", - "changes": [ - { - "addCheckConstraint": { - "constraintBody": "supplier_id BETWEEN 100 and 9999", - "constraintName": "CHECK_SUPPLIER_ID", - "disabled": false, - "tableName": "SUPPLIERS" - } - }] - - } - }, - - { - "changeSet": { - "id": "1604958377003-15", - "author": "wesley (generated)", - "changes": [ - { - "createFunction": { - "functionName": "FUNC_COMPUTE_TAX", - "path": "objects/function/FUNC_COMPUTE_TAX.sql", - "relativeToChangelogFile": true - } - }] - } } - ]} diff --git a/liquibase-core/src/test/resources/liquibase/test-changelog.yml b/liquibase-core/src/test/resources/liquibase/test-changelog.yml index 81ccb6ed59f..1d78e5362e0 100644 --- a/liquibase-core/src/test/resources/liquibase/test-changelog.yml +++ b/liquibase-core/src/test/resources/liquibase/test-changelog.yml @@ -21,16 +21,6 @@ databaseChangeLog: path: objects/storedprocedure/PROC_HELLO_WORLD.sql procedureName: PROC_HELLO_WORLD relativeToChangelogFile: true -- changeSet: - id: 1604960286751-3 - author: wesley (generated) - changes: - - createTrigger: - disabled: false - path: objects/trigger/TRIGGER1.sql - relativeToChangelogFile: true - tableName: ACCOUNT - triggerName: TRIGGER1 - changeSet: id: 1604960286751-4 author: wesley (generated) @@ -207,21 +197,3 @@ databaseChangeLog: type: VARCHAR2(50 BYTE) tableName: SUPPLIERS1 tablespace: USERS -- changeSet: - id: 1604960286751-14 - author: wesley (generated) - changes: - - addCheckConstraint: - constraintBody: supplier_id BETWEEN 100 and 9999 - constraintName: CHECK_SUPPLIER_ID - disabled: false - tableName: SUPPLIERS -- changeSet: - id: 1604960286751-15 - author: wesley (generated) - changes: - - createFunction: - functionName: FUNC_COMPUTE_TAX - path: objects/function/FUNC_COMPUTE_TAX.sql - relativeToChangelogFile: true -