Skip to content

Commit

Permalink
refactor ValidatingVisitor to use factory pattern, error if strict is…
Browse files Browse the repository at this point in the history
… set and defaults file does not exist (DAT-15920) (#5814)
  • Loading branch information
StevenMassaro committed Apr 29, 2024
1 parent 180da16 commit e04fc1e
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions liquibase-standard/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public ValidatingVisitor() {

public ValidatingVisitor(List<RanChangeSet> ranChangeSets) {
ranIndex = new HashMap<>();
for(RanChangeSet changeSet:ranChangeSets) {
for(RanChangeSet changeSet: ranChangeSets) {
ranIndex.put(changeSet.toString(), changeSet);
}
}
Expand Down Expand Up @@ -116,39 +116,11 @@ public void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Data
}

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) &&
Expand All @@ -166,7 +138,48 @@ public void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Data
} 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());
Expand All @@ -191,5 +204,4 @@ public boolean validationPassed() {
duplicateChangeSets.isEmpty() && changeValidationExceptions.isEmpty() && setupExceptions.isEmpty() &&
!validationErrors.hasErrors();
}

}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit e04fc1e

Please sign in to comment.