refactor ValidatingVisitor to use factory pattern, error if strict is set and defaults file does not exist (DAT-15920)

liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java

@@ -12,10 +12,7 @@
 import liquibase.configuration.ConfiguredValue;
 import liquibase.configuration.LiquibaseConfiguration;
 import liquibase.configuration.core.DefaultsFileValueProvider;
-import liquibase.exception.CommandLineParsingException;
-import liquibase.exception.CommandValidationException;
-import liquibase.exception.ExitCodeException;
-import liquibase.exception.LiquibaseException;
+import liquibase.exception.*;
 import liquibase.integration.IntegrationDetails;
 import liquibase.license.LicenseInfo;
 import liquibase.license.LicenseService;
@@ -47,17 +44,12 @@
 import java.security.PrivilegedAction;
 import java.time.Duration;
 import java.util.*;
-import java.util.jar.JarFile;
-import java.util.jar.Manifest;
 import java.util.logging.*;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import java.util.zip.ZipEntry;
 
 import static java.util.ResourceBundle.getBundle;
 import static liquibase.configuration.LiquibaseConfiguration.REGISTERED_VALUE_PROVIDERS_KEY;
-import static liquibase.integration.commandline.LiquibaseLauncherSettings.LiquibaseLauncherSetting.LIQUIBASE_HOME;
-import static liquibase.integration.commandline.LiquibaseLauncherSettings.getSetting;
 import static liquibase.integration.commandline.VersionUtils.*;
 import static liquibase.util.SystemUtil.isWindows;
 
@@ -626,31 +618,36 @@ private List<ConfigurationValueProvider> registerValueProviders(String[] args) t
         }
 
         final PathHandlerFactory pathHandlerFactory = Scope.getCurrentScope().getSingleton(PathHandlerFactory.class);
-        Resource resource = pathHandlerFactory.getResource(defaultsFileConfig.getValue());
+        String defaultsFileConfigValue = defaultsFileConfig.getValue();
+        Resource resource = pathHandlerFactory.getResource(defaultsFileConfigValue);
         if (resource.exists()) {
             try (InputStream defaultsStream = resource.openInputStream()) {
                 if (defaultsStream != null) {
-                    final DefaultsFileValueProvider fileProvider = new DefaultsFileValueProvider(defaultsStream, "File exists at path " + defaultsFileConfig.getValue());
+                    final DefaultsFileValueProvider fileProvider = new DefaultsFileValueProvider(defaultsStream, "File exists at path " + defaultsFileConfigValue);
                     liquibaseConfiguration.registerProvider(fileProvider);
                     returnList.add(fileProvider);
                 }
             }
         } else {
-            InputStream inputStreamOnClasspath = Thread.currentThread().getContextClassLoader().getResourceAsStream(defaultsFileConfig.getValue());
+            InputStream inputStreamOnClasspath = Thread.currentThread().getContextClassLoader().getResourceAsStream(defaultsFileConfigValue);
             if (inputStreamOnClasspath == null) {
-                Scope.getCurrentScope().getLog(getClass()).fine("Cannot find defaultsFile " + defaultsFileConfig.getValue());
+                Scope.getCurrentScope().getLog(getClass()).fine("Cannot find defaultsFile " + defaultsFileConfigValue);
                 if (!defaultsFileConfig.wasDefaultValueUsed()) {
                             //can't use UI since it's not configured correctly yet
-                    System.err.println("Could not find defaults file " + defaultsFileConfig.getValue());
+                    if (GlobalConfiguration.STRICT.getCurrentValue()) {
+                        throw new UnexpectedLiquibaseException("ERROR: The file '"+defaultsFileConfigValue+"' was not found. The global argument 'strict' is enabled, which validates the existence of files specified in liquibase files, such as changelogs, flowfiles, checks packages files, and more. To prevent this message, check your configurations, or disable the 'strict' setting.");
+                    } else {
+                        System.err.println("Could not find defaults file " + defaultsFileConfigValue);
+                    }
                 }
             } else {
-                final DefaultsFileValueProvider fileProvider = new DefaultsFileValueProvider(inputStreamOnClasspath, "File in classpath " + defaultsFileConfig.getValue());
+                final DefaultsFileValueProvider fileProvider = new DefaultsFileValueProvider(inputStreamOnClasspath, "File in classpath " + defaultsFileConfigValue);
                 liquibaseConfiguration.registerProvider(fileProvider);
                 returnList.add(fileProvider);
             }
         }
 
-        final File defaultsFile = new File(defaultsFileConfig.getValue());
+        final File defaultsFile = new File(defaultsFileConfigValue);
         File localDefaultsFile = new File(defaultsFile.getAbsolutePath().replaceFirst(".properties$", ".local.properties"));
         if (localDefaultsFile.exists()) {
             final DefaultsFileValueProvider fileProvider = new DefaultsFileValueProvider(localDefaultsFile) {

liquibase-standard/pom.xml

@@ -342,6 +342,7 @@
                         <param>liquibase.changeset.ChangeSetService</param>
                         <param>liquibase.parser.LiquibaseSqlParser</param>
                         <param>liquibase.database.LiquibaseTableNames</param>
+                        <param>liquibase.changelog.visitor.ValidatingVisitorGenerator</param>
                         <param>liquibase.io.OutputFileHandler</param>
                     </services>
                 </configuration>

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

@@ -7,6 +7,8 @@
 import liquibase.changelog.filter.DbmsChangeSetFilter;
 import liquibase.changelog.filter.LabelChangeSetFilter;
 import liquibase.changelog.visitor.ValidatingVisitor;
+import liquibase.changelog.visitor.ValidatingVisitorGenerator;
+import liquibase.changelog.visitor.ValidatingVisitorGeneratorFactory;
 import liquibase.changeset.ChangeSetService;
 import liquibase.changeset.ChangeSetServiceFactory;
 import liquibase.database.Database;
@@ -367,7 +369,9 @@ public void validate(Database database, Contexts contexts, LabelExpression label
                 new LabelChangeSetFilter(labelExpression)
         );
 
-        ValidatingVisitor validatingVisitor = new ValidatingVisitor(database.getRanChangeSetList());
+        ValidatingVisitorGeneratorFactory validatingVisitorGeneratorFactory = Scope.getCurrentScope().getSingleton(ValidatingVisitorGeneratorFactory.class);
+        ValidatingVisitorGenerator generator = validatingVisitorGeneratorFactory.getValidatingVisitorGenerator();
+        ValidatingVisitor validatingVisitor = generator.generateValidatingVisitor(database.getRanChangeSetList());
         validatingVisitor.validate(database, this);
         logIterator.run(validatingVisitor, new RuntimeEnvironment(database, contexts, labelExpression));
 

liquibase-standard/src/main/java/liquibase/changelog/visitor/StandardValidatingVisitorGenerator.java

@@ -0,0 +1,17 @@
+package liquibase.changelog.visitor;
+
+import liquibase.changelog.RanChangeSet;
+
+import java.util.List;
+
+public class StandardValidatingVisitorGenerator implements ValidatingVisitorGenerator {
+    @Override
+    public int getPriority() {
+        return PRIORITY_DEFAULT;
+    }
+
+    @Override
+    public ValidatingVisitor generateValidatingVisitor(List<RanChangeSet> ranChangeSetList) {
+        return new ValidatingVisitor(ranChangeSetList);
+    }
+}

liquibase-standard/src/main/java/liquibase/changelog/visitor/ValidatingVisitor.java

@@ -47,7 +47,7 @@
 
     public ValidatingVisitor(List<RanChangeSet> ranChangeSets) {
         ranIndex = new HashMap<>();
-        for(RanChangeSet changeSet:ranChangeSets) {
+        for(RanChangeSet changeSet: ranChangeSets) {
             ranIndex.put(changeSet.toString(), changeSet);
         }
     }
@@ -116,39 +116,11 @@
         }
 
         for (Change change : changeSet.getChanges()) {
-            try {
-                change.finishInitialization();
-            } catch (SetupException se) {
-                setupExceptions.add(se);
-            }
-
-
-            if(shouldValidate){
-                warnings.addAll(change.warn(database));
-
-                try {
-                    ValidationErrors foundErrors = change.validate(database);
-                    if ((foundErrors != null)) {
-                        if (foundErrors.hasErrors() && (changeSet.getOnValidationFail().equals
-                                (ChangeSet.ValidationFailOption.MARK_RAN))) {
-                            Scope.getCurrentScope().getLog(getClass()).info(
-                                    "Skipping changeset " + changeSet + " due to validation error(s): " +
-                                            StringUtil.join(foundErrors.getErrorMessages(), ", "));
-                            changeSet.setValidationFailed(true);
-                        } else {
-                            if (!foundErrors.getWarningMessages().isEmpty())
-                                Scope.getCurrentScope().getLog(getClass()).warning(
-                                        "Changeset " + changeSet + ": " +
-                                                StringUtil.join(foundErrors.getWarningMessages(), ", "));
-                            validationErrors.addAll(foundErrors, changeSet);
-                        }
-                    }
-                } catch (Exception e) {
-                    changeValidationExceptions.add(e);
-                }
-            }
+            validateChange(changeSet, database, change, shouldValidate);
         }
 
+        additionalValidations(changeSet, database, shouldValidate, ran);
+
         if(ranChangeSet != null) {
             if (!changeSet.isCheckSumValid(ranChangeSet.getLastCheckSum()) &&
                 !ValidatingVisitorUtil.isChecksumIssue(changeSet, ranChangeSet, databaseChangeLog, database) &&
@@ -166,7 +138,48 @@
         } else {
             seenChangeSets.add(changeSetString);
         }
-    } // public void visit(...)
+    }
+
+    /**
+     * Other implementations of this class might optionally provide additional validations to do in this method.
+     */
+    protected void additionalValidations(ChangeSet changeSet, Database database, boolean shouldValidate, boolean ran) {
+        // purposefully empty
+    }
+
+    protected void validateChange(ChangeSet changeSet, Database database, Change change, boolean shouldValidate) {
+        try {
+            change.finishInitialization();
+        } catch (SetupException se) {
+            setupExceptions.add(se);
+        }
+
+
+        if(shouldValidate){
+            warnings.addAll(change.warn(database));
+
+            try {
+                ValidationErrors foundErrors = change.validate(database);
+                if ((foundErrors != null)) {
+                    if (foundErrors.hasErrors() && (changeSet.getOnValidationFail().equals
+                            (ChangeSet.ValidationFailOption.MARK_RAN))) {
+                        Scope.getCurrentScope().getLog(getClass()).info(
+                                "Skipping changeset " + changeSet + " due to validation error(s): " +
+                                        StringUtil.join(foundErrors.getErrorMessages(), ", "));
+                        changeSet.setValidationFailed(true);
+                    } else {
+                        if (!foundErrors.getWarningMessages().isEmpty())
+                            Scope.getCurrentScope().getLog(getClass()).warning(
+                                    "Changeset " + changeSet + ": " +
+                                            StringUtil.join(foundErrors.getWarningMessages(), ", "));
+                        validationErrors.addAll(foundErrors, changeSet);
+                    }
+                }
+            } catch (Exception e) {
+                changeValidationExceptions.add(e);
+            }
+        }
+    }
 
     private boolean areChangeSetAttributesValid(ChangeSet changeSet) {
         boolean authorEmpty = StringUtil.isEmpty(changeSet.getAuthor());
@@ -191,5 +204,4 @@
             duplicateChangeSets.isEmpty() && changeValidationExceptions.isEmpty() && setupExceptions.isEmpty() &&
             !validationErrors.hasErrors();
     }
-
 }

liquibase-standard/src/main/java/liquibase/changelog/visitor/ValidatingVisitorGenerator.java

@@ -0,0 +1,22 @@
+package liquibase.changelog.visitor;
+
+import liquibase.changelog.RanChangeSet;
+import liquibase.plugin.Plugin;
+
+import java.util.List;
+
+/**
+ * An interface for generating validating visitors, which are used to validate changesets.
+ */
+public interface ValidatingVisitorGenerator extends Plugin {
+
+    int getPriority();
+
+    /**
+     * Generates a validating visitor for the provided list of ran change sets.
+     *
+     * @param ranChangeSetList The list of ran change sets to validate.
+     * @return A validating visitor for the provided list of ran change sets.
+     */
+    ValidatingVisitor generateValidatingVisitor(List<RanChangeSet> ranChangeSetList);
+}

liquibase-standard/src/main/java/liquibase/changelog/visitor/ValidatingVisitorGeneratorFactory.java

@@ -0,0 +1,22 @@
+package liquibase.changelog.visitor;
+
+import liquibase.plugin.AbstractPluginFactory;
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class ValidatingVisitorGeneratorFactory extends AbstractPluginFactory<ValidatingVisitorGenerator> {
+    @Override
+    protected Class<ValidatingVisitorGenerator> getPluginClass() {
+        return ValidatingVisitorGenerator.class;
+    }
+
+    @Override
+    protected int getPriority(ValidatingVisitorGenerator obj, Object... args) {
+        return obj.getPriority();
+    }
+
+    public ValidatingVisitorGenerator getValidatingVisitorGenerator() {
+        return getPlugin();
+    }
+}

liquibase-standard/src/main/java/liquibase/util/ValidatingVisitorUtil.java

@@ -10,6 +10,7 @@
 import liquibase.change.core.DropIndexChange;
 import liquibase.change.core.SQLFileChange;
 import liquibase.changelog.*;
+import liquibase.changelog.visitor.ValidatingVisitor;
 import liquibase.database.Database;
 import liquibase.exception.DatabaseException;
 import liquibase.exception.UnexpectedLiquibaseException;
@@ -20,7 +21,7 @@
 import java.util.stream.Collectors;
 
 /**
- * Util class to offload methods that are used by {@link liquibase.changelog.visitor.ValidatingVisitor} class
+ * Util class to offload methods that are used by {@link ValidatingVisitor} class
  * and may make it more complex than it should be
  */
 public class ValidatingVisitorUtil {

liquibase-standard/src/test/groovy/liquibase/exception/ValidationFailedExceptionTest.groovy

@@ -7,9 +7,9 @@ import liquibase.changelog.DatabaseChangeLog
 import liquibase.changelog.RanChangeSet
 import liquibase.changelog.visitor.ValidatingVisitor
 import liquibase.database.Database
+import liquibase.database.core.MockDatabase
 import liquibase.precondition.core.NotPrecondition
 import liquibase.precondition.core.PreconditionContainer
-import liquibase.database.core.MockDatabase
 import spock.lang.Specification
 
 /**

liquibase-standard/src/test/java/liquibase/changelog/ChangeLogIteratorTest.java

@@ -18,9 +18,7 @@
 import java.util.List;
 import java.util.Set;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 public class ChangeLogIteratorTest {
     private DatabaseChangeLog changeLog;