Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate formatted SQL changelog for invalid patterns DAT-7721 #2761

Merged
merged 12 commits into from
Apr 25, 2022
Merged

Validate formatted SQL changelog for invalid patterns DAT-7721 #2761

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
41dacef
WIP
wwillard7800 Apr 12, 2022
c30dddf
Validate formatted SQL for invalid patterns
wwillard7800 Apr 14, 2022
a12b463
Fix copy and paste error in test
wwillard7800 Apr 14, 2022
93f0c80
Merge remote-tracking branch 'origin/master' into DAT-7721
wwillard7800 Apr 14, 2022
8f73331
Merge remote-tracking branch 'origin/master' into DAT-7721
wwillard7800 Apr 19, 2022
3ba0252
Rework for issues found during test
wwillard7800 Apr 19, 2022
baf9953
Resolve test failure
wwillard7800 Apr 20, 2022
b63b827
Additional cleanup work DAT-7721
wwillard7800 Apr 21, 2022
aee0557
Handle formatted SQL where changeset doesn't have all the identification
wwillard7800 Apr 21, 2022
1aa93a9
Added additional test DAT-7721
wwillard7800 Apr 21, 2022
5359219
Test correction DAT-7721
wwillard7800 Apr 21, 2022
b0fc790
Handle change set lines with invalid attributes
wwillard7800 Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 65 additions & 5 deletions ...se-core/src/main/java/liquibase/parser/core/formattedsql/FormattedSqlChangeLogParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,27 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
RawSQLChange change = null;
Pattern changeLogPattern = Pattern.compile("\\-\\-\\s*liquibase formatted.*", Pattern.CASE_INSENSITIVE);
Pattern propertyPattern = Pattern.compile("\\s*\\-\\-[\\s]*property\\s+(.*:.*)\\s+(.*:.*).*", Pattern.CASE_INSENSITIVE);
Pattern altPropertyOneDashPattern = Pattern.compile("^\\s*?[-]+.*property\\s.*", Pattern.CASE_INSENSITIVE);
Pattern changeSetPattern = Pattern.compile("\\s*\\-\\-[\\s]*changeset\\s+(\"[^\"]+\"|[^:]+):\\s*(\"[^\"]+\"|\\S+).*", Pattern.CASE_INSENSITIVE);
Pattern altChangeSetOneDashPattern = Pattern.compile("^\\s*?[-]+.*changeset\\s.*", Pattern.CASE_INSENSITIVE);
Pattern altChangeSetNoOtherInfoPattern = Pattern.compile("\\s*?\\-\\-[\\s]*?changeset[\\s]*?$", Pattern.CASE_INSENSITIVE);
Pattern rollbackPattern = Pattern.compile("\\s*\\-\\-[\\s]*rollback (.*)", Pattern.CASE_INSENSITIVE);
Pattern altRollbackOneDashPattern = Pattern.compile("^\\s*?[-]+.*?rollback\\s.*", Pattern.CASE_INSENSITIVE);
Pattern preconditionsPattern = Pattern.compile("\\s*\\-\\-[\\s]*preconditions(.*)", Pattern.CASE_INSENSITIVE);
Pattern altPreconditionsOneDashPattern = Pattern.compile("^[-]+.*preconditions\\s.*", Pattern.CASE_INSENSITIVE);
Pattern preconditionPattern = Pattern.compile("\\s*\\-\\-[\\s]*precondition\\-([a-zA-Z0-9-]+) (.*)", Pattern.CASE_INSENSITIVE);
Pattern altPreconditionOneDashPattern = Pattern.compile("^\\s*?[-]+.*precondition\\s.*", Pattern.CASE_INSENSITIVE);
Pattern stripCommentsPattern = Pattern.compile(".*stripComments:(\\w+).*", Pattern.CASE_INSENSITIVE);
Pattern splitStatementsPattern = Pattern.compile(".*splitStatements:(\\w+).*", Pattern.CASE_INSENSITIVE);
Pattern rollbackSplitStatementsPattern = Pattern.compile(".*rollbackSplitStatements:(\\w+).*", Pattern.CASE_INSENSITIVE);
Pattern endDelimiterPattern = Pattern.compile(".*endDelimiter:(\\S*).*", Pattern.CASE_INSENSITIVE);
Pattern rollbackEndDelimiterPattern = Pattern.compile(".*rollbackEndDelimiter:(\\S*).*", Pattern.CASE_INSENSITIVE);
Pattern commentPattern = Pattern.compile("\\-\\-[\\s]*comment:? (.*)", Pattern.CASE_INSENSITIVE);
Pattern altCommentOneDashPattern = Pattern.compile("^\\-[\\s]*comment(.*)$", Pattern.CASE_INSENSITIVE);
Pattern validCheckSumPattern = Pattern.compile("\\-\\-[\\s]*validCheckSum:? (.*)", Pattern.CASE_INSENSITIVE);
Pattern altValidCheckSumOneDashPattern = Pattern.compile("^\\-[\\s]*validCheckSum(.*)$", Pattern.CASE_INSENSITIVE);
Pattern ignoreLinesPattern = Pattern.compile("\\-\\-[\\s]*ignoreLines:(\\w+)", Pattern.CASE_INSENSITIVE);
Pattern altIgnoreLinesOneDashPattern = Pattern.compile("^\\-[\\s]*ignoreLines(.*)$", Pattern.CASE_INSENSITIVE);
Pattern runWithPattern = Pattern.compile(".*runWith:([\\w\\$\\{\\}]+).*", Pattern.CASE_INSENSITIVE);

Pattern runOnChangePattern = Pattern.compile(".*runOnChange:(\\w+).*", Pattern.CASE_INSENSITIVE);
Expand All @@ -114,26 +123,34 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
boolean rollbackSplitStatements = true;
String rollbackEndDelimiter = null;

int count = 0;
String line;
while ((line = reader.readLine()) != null) {
count++;
Matcher propertyPatternMatcher = propertyPattern.matcher(line);
Matcher altPropertyPatternMatcher = altPropertyOneDashPattern.matcher(line);
if (propertyPatternMatcher.matches()) {
handleProperty(changeLogParameters, changeLog, propertyPatternMatcher);
continue;
} else if (altPropertyPatternMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--property name=<property name> value=<property value>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}
Matcher changeLogPatterMatcher = changeLogPattern.matcher (line);
if (changeLogPatterMatcher.matches ()) {
Matcher logicalFilePathMatcher = logicalFilePathPattern.matcher (line);
changeLog.setLogicalFilePath (parseString(logicalFilePathMatcher));
Matcher logicalFilePathMatcher = logicalFilePathPattern.matcher (line);
changeLog.setLogicalFilePath (parseString(logicalFilePathMatcher));

Matcher changeLogIdMatcher = changeLogIdPattern.matcher (line);
changeLog.setChangeLogId (parseString(changeLogIdMatcher));
}

Matcher ignoreLinesMatcher = ignoreLinesPattern.matcher(line);
Matcher altIgnoreLinesOneDashMatcher = altIgnoreLinesOneDashPattern.matcher(line);
if (ignoreLinesMatcher.matches ()) {
if ("start".equals(ignoreLinesMatcher.group(1))){
while ((line = reader.readLine()) != null){
count++;
ignoreLinesMatcher = ignoreLinesPattern.matcher(line);
if (ignoreLinesMatcher.matches ()) {
if ("end".equals(ignoreLinesMatcher.group(1))){
Expand All @@ -147,12 +164,16 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
long ignoreCount = Long.parseLong(ignoreLinesMatcher.group(1));
while ( ignoreCount>0 && (line = reader.readLine()) != null){
ignoreCount--;
count++;
}
continue;
} catch (NumberFormatException | NullPointerException nfe) {
throw new ChangeLogParseException("Unknown ignoreLines syntax");
}
}
} else if (altIgnoreLinesOneDashMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--ignore start' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}

Matcher changeSetPatternMatcher = changeSetPattern.matcher(line);
Expand Down Expand Up @@ -257,26 +278,59 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
currentSql.setLength(0);
currentRollbackSql.setLength(0);
} else {
Matcher altChangeSetOneDashPatternMatcher = altChangeSetOneDashPattern.matcher(line);
Matcher altChangeSetNoOtherInfoPatternMatcher = altChangeSetNoOtherInfoPattern.matcher(line);
if (altChangeSetOneDashPatternMatcher.matches() || altChangeSetNoOtherInfoPatternMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--changeset <authorname>:<changesetId>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}
if (changeSet != null) {
Matcher commentMatcher = commentPattern.matcher(line);
Matcher altCommentOneDashMatcher = altCommentOneDashPattern.matcher(line);
Matcher rollbackMatcher = rollbackPattern.matcher(line);
Matcher altRollbackMatcher = altRollbackOneDashPattern.matcher(line);
Matcher preconditionsMatcher = preconditionsPattern.matcher(line);
Matcher altPreconditionsOneDashMatcher = altPreconditionsOneDashPattern.matcher(line);
Matcher preconditionMatcher = preconditionPattern.matcher(line);
Matcher altPreconditionOneDashMatcher = altPreconditionOneDashPattern.matcher(line);
Matcher validCheckSumMatcher = validCheckSumPattern.matcher(line);
Matcher altValidCheckSumOneDashMatcher = altValidCheckSumOneDashPattern.matcher(line);

if (commentMatcher.matches()) {
if (commentMatcher.groupCount() == 0) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--comment <comment>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}
if (commentMatcher.groupCount() == 1) {
changeSet.setComments(commentMatcher.group(1));
}
} else if (altCommentOneDashMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--comment <comment>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else if (validCheckSumMatcher.matches()) {
if (validCheckSumMatcher.groupCount() == 1) {
if (validCheckSumMatcher.groupCount() == 0) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--rollback <rollback SQL>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else if (validCheckSumMatcher.groupCount() == 1) {
changeSet.addValidCheckSum(validCheckSumMatcher.group(1));
}
} else if (altValidCheckSumOneDashMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--validChecksum <checksum>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else if (rollbackMatcher.matches()) {
if (rollbackMatcher.groupCount() == 1) {
currentRollbackSql.append(rollbackMatcher.group(1)).append(System.lineSeparator());
if (rollbackMatcher.groupCount() == 0) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--rollback <rollback SQL>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}
currentRollbackSql.append(rollbackMatcher.group(1)).append(System.lineSeparator());
} else if (altRollbackMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--rollback <rollback SQL>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else if (preconditionsMatcher.matches()) {
if (preconditionsMatcher.groupCount() == 0) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--preconditions <onFail>|<onError>|<onUpdate>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
}
if (preconditionsMatcher.groupCount() == 1) {
String body = preconditionsMatcher.group(1);
Matcher onFailMatcher = onFailPattern.matcher(body);
Expand All @@ -289,6 +343,9 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
pc.setOnSqlOutput(StringUtil.trimToNull(parseString(onUpdateSqlMatcher)));
changeSet.setPreconditions(pc);
}
} else if (altPreconditionsOneDashMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--preconditions <onFail>|<onError>|<onUpdate>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else if (preconditionMatcher.matches()) {
if (changeSet.getPreconditions() == null) {
// create the defaults
Expand All @@ -305,6 +362,9 @@ public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParame
}
}
}
} else if (altPreconditionOneDashMatcher.matches()) {
String message = String.format("Unexpected formatting at line %d. Formatted SQL changelogs require known formats, such as '--precondition <SqlCheck>' and others to be recognized and run. Learn all the options at https://docs.liquibase.com/concepts/changelogs/sql-format.html", count);
throw new ChangeLogParseException("\n" + message);
} else {
currentSql.append(line).append(System.lineSeparator());
}
Expand Down
77 changes: 76 additions & 1 deletion ...src/test/groovy/liquibase/parser/core/formattedsql/FormattedSqlChangeLogParserTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,20 @@ grant execute on any_procedure_name to ANY_USER3/
"""

private static final String INVALID_CHANGELOG = "select * from table1"
private static final String INVALID_CHANGELOG_INVALID_PRECONDITION = "--liquibase formatted sql\n" +
private static final String INVALID_CHANGELOG_INVALID_PRECONDITION =
"--liquibase formatted sql\n" +
"\n" +
"--changeset bboisvert:invalid_precondition\n" +
"--precondition-invalid-type 123\n" +
"select 1;"

private static final String INVALID_CHANGELOG_INVALID_PRECONDITION_PATTERN =
"--liquibase formatted sql\n" +
"\n" +
"--changeset bboisvert:invalid_precondition\n" +
"-precondition 123\n" +
"select 1;"

def supports() throws Exception {
expect:
assert new MockFormattedSqlChangeLogParser(VALID_CHANGELOG).supports("asdf.sql", new JUnitResourceAccessor())
Expand All @@ -131,6 +139,16 @@ grant execute on any_procedure_name to ANY_USER3/
thrown(ChangeLogParseException)
}

def invalidPreconditionPattern() throws Exception {
when:
new MockFormattedSqlChangeLogParser(INVALID_CHANGELOG_INVALID_PRECONDITION_PATTERN).parse("asdf.sql", new ChangeLogParameters(), new JUnitResourceAccessor())
then:
def e = thrown(ChangeLogParseException)
assert e != null
assert e instanceof ChangeLogParseException
assert e.getMessage().toLowerCase().contains("--precondition <sqlcheck>")
}

def parse() throws Exception {
expect:
ChangeLogParameters params = new ChangeLogParameters()
Expand Down Expand Up @@ -286,6 +304,51 @@ grant execute on any_procedure_name to ANY_USER3/

}

def parse_changeSetWithOneDash() throws Exception {
when:
String changeLogWithOneDash = "--liquibase formatted sql\n\n" +
"-changeset John Doe:12345\n" +
"create table test (id int);\n"

DatabaseChangeLog changeLog = new MockFormattedSqlChangeLogParser(changeLogWithOneDash).parse("asdf.sql", new ChangeLogParameters(), new JUnitResourceAccessor())

then:
thrown(ChangeLogParseException)
}

def parse_rollbackWithOneDash() throws Exception {
when:
String changeLogWithOneDash =
"--liquibase formatted sql\n\n" +
"--changeset John Doe:12345\n" +
"create table test (id int);\n" +
"-rollback drop table test;\n"

DatabaseChangeLog changeLog = new MockFormattedSqlChangeLogParser(changeLogWithOneDash).parse("asdf.sql", new ChangeLogParameters(), new JUnitResourceAccessor())

then:
def e = thrown(ChangeLogParseException)
assert e instanceof ChangeLogParseException
assert e.getMessage().toLowerCase().contains("--rollback <rollback sql>")
}

def parse_propertykWithOneDash() throws Exception {
when:
String changeLogWithOneDash =
"--liquibase formatted sql\n\n" +
"-property name=foo value=bar\n" +
"--changeset John Doe:12345\n" +
"create table test (id int);\n" +
"-rollback drop table test;\n"

DatabaseChangeLog changeLog = new MockFormattedSqlChangeLogParser(changeLogWithOneDash).parse("asdf.sql", new ChangeLogParameters(), new JUnitResourceAccessor())

then:
def e = thrown(ChangeLogParseException)
assert e instanceof ChangeLogParseException
assert e.getMessage().toLowerCase().contains("-property name")
}

def parse_withComment() throws Exception {
when:
String changeLogWithComment = "--liquibase formatted sql\n\n" +
Expand All @@ -302,6 +365,18 @@ grant execute on any_procedure_name to ANY_USER3/
changeLog.getChangeSets().get(0).getComments() == "This is a test comment"
}

def parse_withCommentThatDoesNotMatch() {
when:
String changeLogWithComment = "--liquibase formatted sql\n\n" +
"--changeset JohnDoe:12345\n" +
"-comment: This is a test comment\n" +
"create table test (id int);\n"
DatabaseChangeLog changeLog = new MockFormattedSqlChangeLogParser(changeLogWithComment).parse("asdf.sql", new ChangeLogParameters(), new JUnitResourceAccessor())

then:
thrown(ChangeLogParseException)
}

@Unroll
def parse_multipleDbms() throws Exception {
when:
Expand Down