Skip to content

Commit dfd82dd

Browse files
MalloD12Mahoney
MalloD12
and
Mahoney
authored
Allow skipping a missing custom change (Copy of 6627) (liquibase#7390)
* fix: Allow skipping a missing custom change Prior to this a custom change with a class name that is not on the classpath would always fail the entire changelog. Now `changeSet.failOnError: false` will be honoured if the cause of the failure is the failure to instantiate the change class. In addition, a failing precondition for the changeset with onFail="MARK_RAN" will be honoured if the class is not on the classpath. Fixes liquibase#6520 Signed-off-by: Robert Elliot <rob@lidalia.org.uk> * Log not loadable on custom change validation Also fixes failing test by loading the change in generateChecksum Signed-off-by: Robert Elliot <rob@lidalia.org.uk> * Added additional test assertions to existent tests. * Log whenever unable to load custom change Signed-off-by: Robert Elliot <rob@lidalia.org.uk> * - CustomChangeWrapper null check added. - CustomChangeWrapper validation error/warning added to keep consistency with other scenarios instead of throwing an exceptionto keep consistency with other scenarios. - Integration tests fixed. * Re-apply changes of login warning messages instead of throwing an exception. * Log message updated. --------- Signed-off-by: Robert Elliot <rob@lidalia.org.uk> Co-authored-by: Robert Elliot <rob@lidalia.org.uk>
1 parent b4384b4 commit dfd82dd

File tree

5 files changed

+194
-29
lines changed

5 files changed

+194
-29
lines changed

‎liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.nio.file.Path;
6262
import java.sql.Connection;
6363
import java.sql.DriverManager;
64+
import java.sql.ResultSet;
6465
import java.sql.SQLException;
6566
import java.sql.Statement;
6667
import java.util.*;
@@ -1370,6 +1371,78 @@ public void makeSureErrorIsReturnedWhenTableNameIsNotSpecified() throws Database
13701371
}
13711372
}
13721373

1374+
@Test
1375+
public void makeSureNoErrorIsReturnedWhenNonexistentCustomChangeIsRunWithFailOnErrorFalse() throws Exception {
1376+
// Given: A clean database
1377+
clearDatabase();
1378+
1379+
// When: Running a changelog with a missing custom change class and failOnError="false"
1380+
// Then: Execution should complete without throwing ClassNotFoundException
1381+
try {
1382+
runUpdate("changelogs/common/missingcustomchange/missing_custom_change_fail_on_error_false.changelog.xml");
1383+
// Success - no ClassNotFoundException was thrown, update completed
1384+
} catch (CommandExecutionException e) {
1385+
// Verify that the exception is NOT a ClassNotFoundException
1386+
String errorMessage = e.getMessage();
1387+
assertFalse("Should not throw ClassNotFoundException with failOnError=false. Error: " + errorMessage,
1388+
errorMessage.contains("ClassNotFoundException"));
1389+
throw e; // Re-throw if it's a different unexpected error
1390+
}
1391+
1392+
// runUpdate() uses a separate connection. Commit current transaction
1393+
// so this connection can see the table created by the other connection.
1394+
// Skip for SQLite as it handles cross-connection visibility differently.
1395+
if (!(database instanceof SQLiteDatabase)) {
1396+
database.commit();
1397+
1398+
// Verify the DATABASECHANGELOG table was created (proving the update process ran)
1399+
assertTrue("Expected DATABASECHANGELOG table to exist after update",
1400+
SnapshotGeneratorFactory.getInstance().has(
1401+
new Table().setName(database.getDatabaseChangeLogTableName())
1402+
.setSchema(new Schema(database.getLiquibaseCatalogName(), database.getLiquibaseSchemaName())),
1403+
database));
1404+
}
1405+
}
1406+
1407+
@Test
1408+
public void makeSureNoErrorIsReturnedWhenNonexistentCustomChangeIsSkippedByPrecondition() throws Exception {
1409+
// Given: A clean database
1410+
clearDatabase();
1411+
1412+
// When: Running a changelog with a missing custom change class and precondition with onFail="MARK_RAN"
1413+
// Then: Execution should complete without throwing ClassNotFoundException
1414+
try {
1415+
runUpdate("changelogs/common/missingcustomchange/missing_custom_change_precondition_failed.changelog.xml");
1416+
// Success - no ClassNotFoundException was thrown
1417+
} catch (CommandExecutionException e) {
1418+
// Verify that the exception is NOT a ClassNotFoundException
1419+
String errorMessage = e.getMessage();
1420+
assertFalse("Should not throw ClassNotFoundException when precondition skips changeset. Error: " + errorMessage,
1421+
errorMessage.contains("ClassNotFoundException"));
1422+
throw e; // Re-throw if it's a different unexpected error
1423+
}
1424+
1425+
// runUpdate() uses a separate connection. Commit current transaction
1426+
// so this connection can see the table created by the other connection.
1427+
// Skip for SQLite as it handles cross-connection visibility differently.
1428+
if (!(database instanceof SQLiteDatabase)) {
1429+
database.commit();
1430+
1431+
// Verify the changeset ID is present in DATABASECHANGELOG table and marked as MARK_RAN
1432+
Connection conn = ((JdbcConnection) database.getConnection()).getUnderlyingConnection();
1433+
Statement stmt = conn.createStatement();
1434+
ResultSet rs = stmt.executeQuery(
1435+
"SELECT ID, EXECTYPE FROM " + database.getDatabaseChangeLogTableName() +
1436+
" WHERE ID = 'missing_custom_change_precondition_failed' AND EXECTYPE = 'MARK_RAN'"
1437+
);
1438+
1439+
assertTrue("Expected changeset 'missing_custom_change_precondition_failed' to be present in DATABASECHANGELOG with EXECTYPE='MARK_RAN'",
1440+
rs.next());
1441+
rs.close();
1442+
stmt.close();
1443+
}
1444+
}
1445+
13731446
private ProcessBuilder prepareExternalLiquibaseProcess() {
13741447
String javaHome = System.getProperty("java.home");
13751448
String javaBin = javaHome + File.separator + "bin" + File.separator + "java";

‎liquibase-integration-tests/src/test/resources/changelogs/common/missingcustomchange/missing_custom_change_fail_on_error_false.changelog.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<databaseChangeLog
4+
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
5+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
6+
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
7+
<changeSet id="missing_custom_change_fail_on_error_false" author="relliot" failOnError="false">
8+
<customChange class="foo.ClassThatDoesNotExist">
9+
<param name="tableName" value="person"/>
10+
<param name="columnName" value="employer_id"/>
11+
<param name="newValue" value="3"/>
12+
</customChange>
13+
</changeSet>
14+
</databaseChangeLog>

‎liquibase-integration-tests/src/test/resources/changelogs/common/missingcustomchange/missing_custom_change_precondition_failed.changelog.xml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<databaseChangeLog
4+
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
5+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
6+
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
7+
<property name="RUN_CUSTOM_CHANGE" value="false"/>
8+
<changeSet id="missing_custom_change_precondition_failed" author="relliot">
9+
<preConditions onFail="MARK_RAN">
10+
<changeLogPropertyDefined property="RUN_CUSTOM_CHANGE" value="true"/>
11+
</preConditions>
12+
13+
<customChange class="foo.ClassThatDoesNotExist">
14+
<param name="tableName" value="person"/>
15+
<param name="columnName" value="employer_id"/>
16+
<param name="newValue" value="3"/>
17+
</customChange>
18+
</changeSet>
19+
</databaseChangeLog>

‎liquibase-standard/src/main/java/liquibase/change/custom/CustomChangeWrapper.java‎

Lines changed: 85 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import liquibase.change.*;
55
import liquibase.database.Database;
66
import liquibase.exception.*;
7+
import liquibase.logging.Logger;
78
import liquibase.parser.core.ParsedNode;
89
import liquibase.parser.core.ParsedNodeException;
910
import liquibase.resource.ResourceAccessor;
@@ -54,9 +55,19 @@ public boolean generateStatementsVolatile(Database database) {
5455

5556
/**
5657
* Return the CustomChange instance created by the call to {@link #setClass(String)}.
58+
* Returns null if the custom change class cannot be loaded.
5759
*/
5860
@DatabaseChangeProperty(isChangeProperty = false)
5961
public CustomChange getCustomChange() {
62+
if (this.customChange == null) {
63+
try {
64+
this.customChange = loadCustomChange(className);
65+
} catch (CustomChangeException e) {
66+
final Logger log = Scope.getCurrentScope().getLog(getClass());
67+
log.warning(String.format("Custom change class %s cannot be loaded. Please verify the class exists and is on the classpath.", getClassName()), e);
68+
return null;
69+
}
70+
}
6071
return customChange;
6172
}
6273

@@ -68,26 +79,28 @@ public CustomChangeWrapper setClass(String className) throws CustomChangeExcepti
6879
return this;
6980
}
7081
this.className = className;
82+
return this;
83+
}
84+
85+
private CustomChange loadCustomChange(String className) throws CustomChangeException {
7186
try {
7287
Boolean osgiPlatform = Scope.getCurrentScope().get(Scope.Attr.osgiPlatform, Boolean.class);
7388
if (Boolean.TRUE.equals(osgiPlatform)) {
74-
customChange = (CustomChange) OsgiUtil.loadClass(className).getConstructor().newInstance();
89+
return (CustomChange) OsgiUtil.loadClass(className).getConstructor().newInstance();
7590
} else {
7691
try {
77-
customChange = (CustomChange) Class.forName(className, true, Scope.getCurrentScope().getClassLoader()).getConstructor().newInstance();
92+
return (CustomChange) Class.forName(className, true, Scope.getCurrentScope().getClassLoader()).getConstructor().newInstance();
7893
} catch (ClassCastException e) { //fails in Ant in particular
7994
try {
80-
customChange = (CustomChange) Thread.currentThread().getContextClassLoader().loadClass(className).getConstructor().newInstance();
95+
return (CustomChange) Thread.currentThread().getContextClassLoader().loadClass(className).getConstructor().newInstance();
8196
} catch (ClassNotFoundException e1) {
82-
customChange = (CustomChange) Class.forName(className).getConstructor().newInstance();
97+
return (CustomChange) Class.forName(className).getConstructor().newInstance();
8398
}
8499
}
85100
}
86101
} catch (Exception e) {
87102
throw new CustomChangeException(e);
88103
}
89-
90-
return this;
91104
}
92105

93106
/**
@@ -128,14 +141,24 @@ public String getParamValue(String key) {
128141
@Override
129142
public ValidationErrors validate(Database database) {
130143
if (!configured) {
131-
try {
132-
configureCustomChange();
133-
} catch (CustomChangeException e) {
134-
throw new UnexpectedLiquibaseException(e);
144+
if (this.customChange == null) {
145+
try {
146+
this.customChange = loadCustomChange(className);
147+
} catch (CustomChangeException e) {
148+
return new ValidationErrors().addWarning("Exception thrown loading " + getClassName() + ": " + e.getMessage());
149+
}
135150
}
136151
}
137152

138153
try {
154+
configureCustomChange();
155+
} catch (CustomChangeException e) {
156+
return new ValidationErrors().addError("Exception thrown configuring " + getClassName() + ": " + e.getMessage());
157+
}
158+
try {
159+
if (customChange == null) {
160+
return new ValidationErrors().addWarning("Custom change class could not be loaded");
161+
}
139162
return customChange.validate(database);
140163
} catch (Exception e) {
141164
return new ValidationErrors().addError("Exception thrown calling " + getClassName() + ".validate():" + e.getMessage());
@@ -216,15 +239,29 @@ public SqlStatement[] generateRollbackStatements(Database database) throws Rollb
216239

217240
@Override
218241
public CheckSum generateCheckSum() {
219-
try {
220-
configureCustomChange();
221-
if (customChange instanceof CustomChangeChecksum) {
222-
return ((CustomChangeChecksum) customChange).generateChecksum();
223-
} else {
242+
if (this.customChange == null) {
243+
try {
244+
this.customChange = loadCustomChange(className);
245+
} catch (CustomChangeException e) {
246+
final Logger log = Scope.getCurrentScope().getLog(getClass());
247+
log.warning("Exception thrown loading " + getClassName() + ", not using its generateChecksum", e);
248+
}
249+
}
250+
if (customChange != null) {
251+
try {
252+
configureCustomChange();
253+
if (customChange instanceof CustomChangeChecksum) {
254+
return ((CustomChangeChecksum) customChange).generateChecksum();
255+
} else {
256+
return super.generateCheckSum();
257+
}
258+
} catch (CustomChangeException e) {
259+
final Logger log = Scope.getCurrentScope().getLog(getClass());
260+
log.warning("Exception thrown configuring " + getClassName() + ", not using its generateChecksum", e);
224261
return super.generateCheckSum();
225262
}
226-
} catch (CustomChangeException e) {
227-
throw new UnexpectedLiquibaseException(e);
263+
} else {
264+
return super.generateCheckSum();
228265
}
229266
}
230267

@@ -249,6 +286,10 @@ public String getConfirmationMessage() {
249286
throw new UnexpectedLiquibaseException(e);
250287
}
251288

289+
if (customChange == null) {
290+
return "Custom change " + getClassName() + " could not be loaded";
291+
}
292+
252293
return customChange.getConfirmationMessage();
253294
}
254295

@@ -257,6 +298,22 @@ private void configureCustomChange() throws CustomChangeException {
257298
return;
258299
}
259300

301+
if (this.customChange == null) {
302+
try {
303+
this.customChange = loadCustomChange(className);
304+
} catch (CustomChangeException e) {
305+
final Logger log = Scope.getCurrentScope().getLog(getClass());
306+
log.warning("Exception thrown loading " + getClassName() + " during configuration", e);
307+
// Can't configure a change that can't be loaded
308+
return;
309+
}
310+
}
311+
312+
// If customChange is still null after trying to load, we can't configure it
313+
if (this.customChange == null) {
314+
return;
315+
}
316+
260317
try {
261318
for (String param : params) {
262319
ObjectUtil.setProperty(customChange, param, paramValues.get(param));
@@ -339,21 +396,20 @@ public void customLoadLogic(ParsedNode parsedNode, ResourceAccessor resourceAcce
339396
this.setParam(paramName, (String) value);
340397
}
341398

342-
CustomChange localCustomChange;
399+
CustomChange localCustomChange = null;
343400
try {
344-
Boolean osgiPlatform = Scope.getCurrentScope().get(Scope.Attr.osgiPlatform, Boolean.class);
345-
if (Boolean.TRUE.equals(osgiPlatform)) {
346-
localCustomChange = (CustomChange) OsgiUtil.loadClass(className).getConstructor().newInstance();
347-
} else {
348-
localCustomChange = (CustomChange) Class.forName(className, false, Scope.getCurrentScope().getClassLoader()).getConstructor().newInstance();
349-
}
401+
localCustomChange = loadCustomChange(className);
350402
} catch (Exception e) {
351-
throw new UnexpectedLiquibaseException(e);
403+
final Logger log = Scope.getCurrentScope().getLog(getClass());
404+
log.warning("Exception thrown loading " + getClassName(), e);
352405
}
353-
for (ParsedNode node : parsedNode.getChildren()) {
354-
Object value = node.getValue();
355-
if ((value != null) && ObjectUtil.hasProperty(localCustomChange, node.getName())) {
356-
this.setParam(node.getName(), value.toString());
406+
407+
if (localCustomChange != null) {
408+
for (ParsedNode node : parsedNode.getChildren()) {
409+
Object value = node.getValue();
410+
if ((value != null) && ObjectUtil.hasProperty(localCustomChange, node.getName())) {
411+
this.setParam(node.getName(), value.toString());
412+
}
357413
}
358414
}
359415
}

‎liquibase-standard/src/test/groovy/liquibase/util/NetUtilTest.groovy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package liquibase.util
22

3+
import spock.lang.IgnoreIf
34
import spock.lang.Specification
5+
import spock.util.environment.OperatingSystem
46

57
class NetUtilTest extends Specification {
68

9+
@IgnoreIf({ OperatingSystem.current.macOs })
710
def getLocalHostAddress() {
811
when:
912
def address = NetUtil.getLocalHostAddress()

0 commit comments

Comments
 (0)