Global flag to restore behaviour of not failing on duplicated changeset identifiers

liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy

@@ -19,6 +19,16 @@ Usage: liquibase [GLOBAL OPTIONS] [COMMAND] [COMMAND OPTIONS]
 Command-specific help: "liquibase <command-name> --help"
 
 Global Options
+      --allow-duplicated-changeset-identifiers=PARAM
+                             Allows duplicated changeset identifiers without
+                               failing Liquibase execution.
+                             DEFAULT: false
+                             (defaults file: 'liquibase.
+                               allowDuplicatedChangesetIdentifiers',
+                               environment variable:
+                               'LIQUIBASE_ALLOW_DUPLICATED_CHANGESET_IDENTIFIERS
+                               ')
+
       --always-drop-instead-of-replace=PARAM
                              If true, drop and recreate a view instead of
                                replacing it.

liquibase-standard/src/main/java/liquibase/GlobalConfiguration.java

@@ -36,6 +36,7 @@ public class GlobalConfiguration implements AutoloadedConfigurations {
     public static final ConfigurationDefinition<Boolean> SHOW_BANNER;
     public static final ConfigurationDefinition<Boolean> ALWAYS_DROP_INSTEAD_OF_REPLACE;
     public static final ConfigurationDefinition<DuplicateFileMode> DUPLICATE_FILE_MODE;
+    public static final ConfigurationDefinition<Boolean> ALLOW_DUPLICATED_CHANGESETS_IDENTIFIERS;
 
     public static final ConfigurationDefinition<Boolean> VALIDATE_XML_CHANGELOG_FILES;
 
@@ -243,6 +244,12 @@ public class GlobalConfiguration implements AutoloadedConfigurations {
                 .setValueHandler(ValueHandlerUtil::booleanValueHandler)
                 .build();
 
+        ALLOW_DUPLICATED_CHANGESETS_IDENTIFIERS = builder.define("allowDuplicatedChangesetIdentifiers", Boolean.class)
+                .setDescription("Allows duplicated changeset identifiers without failing Liquibase execution.")
+                .setValueHandler(ValueHandlerUtil::booleanValueHandler)
+                .setDefaultValue(false)
+                .build();
+
         VALIDATE_XML_CHANGELOG_FILES = builder.define("validateXmlChangelogFiles", Boolean.class)
                 .setDescription("Will perform xsd validation of XML changelog files. When many XML changelog files are included this validation may impact Liquibase performance. Defaults to true.")
                 .setValueHandler(ValueHandlerUtil::booleanValueHandler)

liquibase-standard/src/main/java/liquibase/changelog/ChangeLogIterator.java

@@ -1,9 +1,6 @@
 package liquibase.changelog;
 
-import liquibase.ContextExpression;
-import liquibase.Labels;
-import liquibase.RuntimeEnvironment;
-import liquibase.Scope;
+import liquibase.*;
 import liquibase.changelog.filter.ChangeSetFilter;
 import liquibase.changelog.filter.ChangeSetFilterResult;
 import liquibase.changelog.visitor.ChangeSetVisitor;
@@ -14,13 +11,19 @@
 import liquibase.exception.ValidationErrors;
 import liquibase.executor.Executor;
 import liquibase.executor.ExecutorService;
+import liquibase.util.BooleanUtil;
 import liquibase.util.StringUtil;
 import lombok.Getter;
 
 import java.util.*;
 
 import static java.util.ResourceBundle.getBundle;
 
+/**
+ * The ChangeLogIterator class is responsible for iterating through a list of ChangeSets in a DatabaseChangeLog
+ * and executing a visitor for each ChangeSet that passes the specified filters.
+ * It provides methods for running the visitor and validating the Executor for each ChangeSet.
+ */
 public class ChangeLogIterator {
 
     protected final DatabaseChangeLog databaseChangeLog;
@@ -39,6 +42,8 @@ public class ChangeLogIterator {
     @Getter
     private final List<ChangeSet> skippedDueToExceptionChangeSets = new ArrayList<>();
 
+    private final Boolean allowDuplicatedChangesetsIdentifiers = GlobalConfiguration.ALLOW_DUPLICATED_CHANGESETS_IDENTIFIERS.getCurrentValue();
+
     public ChangeLogIterator(DatabaseChangeLog databaseChangeLog, ChangeSetFilter... changeSetFilters) {
         this(databaseChangeLog, Arrays.asList(changeSetFilters));
     }
@@ -115,7 +120,7 @@ public void run() throws Exception {
 
                         int finalI = i;
                         Scope.child(scopeValues, () -> {
-                            if (finalShouldVisit) {
+                            if (finalShouldVisit && !alreadySaw(changeSet)) {
                                 //
                                 // Go validate any changesets with an Executor if
                                 // we are using a ValidatingVisitor
@@ -193,9 +198,24 @@ protected void markSeen(ChangeSet changeSet) {
         if (changeSet.key == null) {
             changeSet.key = createKey(changeSet);
         }
-
         seenChangeSets.add(changeSet.key);
+    }
+
 
+    /**
+     * By default, this returns false as finalShouldVisit logic has better performance.
+     * But prior to 4.19.1 the below logic worked, were if a changeset was already saw we would
+     * just ignore it instead of throwing an error. This logic was backported under flag
+     * ALLOW_DUPLICATED_CHANGESETS_IDENTIFIERS
+     */
+    private boolean alreadySaw(ChangeSet changeSet) {
+        if (BooleanUtil.isTrue(allowDuplicatedChangesetsIdentifiers)) {
+            if (changeSet.key == null) {
+                changeSet.key = createKey(changeSet);
+            }
+            return seenChangeSets.contains(changeSet.key);
+        }
+        return false;
     }
 
     /**
@@ -204,7 +224,6 @@ protected void markSeen(ChangeSet changeSet) {
     protected String createKey(ChangeSet changeSet) {
         Labels labels = changeSet.getLabels();
         ContextExpression contexts = changeSet.getContextFilter();
-        changeSet.getRunOrder();
 
         return changeSet.toString(false)
                 + ":" + (labels == null ? null : labels.toString())

liquibase-standard/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.groovy

@@ -50,29 +50,31 @@ class XMLChangeLogSAXParserTest extends Specification {
 </databaseChangeLog>
 """
 
+    @Unroll
     def testAllProvidedChangesetsAreLoaded() throws ChangeLogParseException, Exception {
-        when:
+        given:
         def xmlParser = new XMLChangeLogSAXParser()
-        def changeLog = xmlParser.parse("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/master.changelog.xml",
+        final def changeLog = xmlParser.parse("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/master.changelog.xml",
                 new ChangeLogParameters(), new JUnitResourceAccessor())
 
-        final List<ChangeSet> changeSets = new ArrayList<ChangeSet>()
-
-        new ChangeLogIterator(changeLog).run(new ChangeSetVisitor() {
-            @Override
-            ChangeSetVisitor.Direction getDirection() {
-                return ChangeSetVisitor.Direction.FORWARD
-            }
-
-            @Override
-            void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Database database, Set<ChangeSetFilterResult> filterResults) throws LiquibaseException {
-                changeSets.add(changeSet)
-            }
-        }, new RuntimeEnvironment(new MockDatabase(), new Contexts(), new LabelExpression()))
+        when:
+        final def changeSets = new ArrayList<ChangeSet>()
+        Scope.child(GlobalConfiguration.ALLOW_DUPLICATED_CHANGESETS_IDENTIFIERS.key, allowDuplicatedChangesetIdentifiers, { ->
+            new ChangeLogIterator(changeLog).run(new ChangeSetVisitor() {
+                @Override
+                ChangeSetVisitor.Direction getDirection() {
+                    return ChangeSetVisitor.Direction.FORWARD
+                }
+                @Override
+                void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Database database, Set<ChangeSetFilterResult> filterResults) throws LiquibaseException {
+                    changeSets.add(changeSet)
+                }
+            }, new RuntimeEnvironment(new MockDatabase(), new Contexts(), new LabelExpression()))
+        })
 
 
         then:
-        changeSets.size() == 14
+        changeSets.size() == totalSize
         changeSets.get(0).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
         changeSets.get(1).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
         changeSets.get(1).getContextFilter().getContexts().size() == 1
@@ -83,9 +85,24 @@ class XMLChangeLogSAXParserTest extends Specification {
         changeSets.get(4).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
         changeSets.get(4).getDbmsSet().size() == 1
         changeSets.get(5).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog1.xml::1::testuser"
-        changeSets.get(6).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
-        changeSets.get(7).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
-        changeSets.get(13).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog3.xml::1::testuser"
+
+        if (!allowDuplicatedChangesetIdentifiers) {
+            changeSets.get(6).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
+            changeSets.get(7).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser"
+            changeSets.get(13).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog3.xml::1::testuser"
+        } else {
+            changeSets.get(6).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog3.xml::1::testuser"
+            changeSets.get(7).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog2.xml::1::testuser"
+        }
+
+
+        where:
+        allowDuplicatedChangesetIdentifiers | totalSize
+        false                               | 14
+        true                                | 8
+
+
+
     }
 
     def "uses liquibase.secureParsing by default"() {