From c31f946ef44f77041ed978b13be33379225e73ad Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Tue, 28 Jun 2022 13:29:28 -0500 Subject: [PATCH 1/5] warn user if XSD version does not match build version --- .../core/xml/LiquibaseEntityResolver.java | 30 +++++++++++++ .../xml/LiquibaseEntityResolverTest.groovy | 42 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java index 8419bfa0d6f..cca0ec2a9e1 100644 --- a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java +++ b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java @@ -6,12 +6,15 @@ import liquibase.resource.ClassLoaderResourceAccessor; import liquibase.resource.InputStreamList; import liquibase.resource.ResourceAccessor; +import liquibase.util.LiquibaseUtil; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.ext.EntityResolver2; import java.io.IOException; import java.io.InputStream; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Finds the Liquibase schema from the classpath rather than fetching it over the Internet. @@ -37,6 +40,8 @@ public InputSource resolveEntity(String name, String publicId, String baseURI, S .replace("http://www.liquibase.org/xml/ns/migrator/", "http://www.liquibase.org/xml/ns/dbchangelog/") .replaceFirst("https?://", ""); + warnForMismatchedXsdVersion(systemId); + ResourceAccessor resourceAccessor = Scope.getCurrentScope().getResourceAccessor(); InputStreamList streams = resourceAccessor.openStreams(null, path); if (streams.isEmpty()) { @@ -70,6 +75,31 @@ public InputSource resolveEntity(String name, String publicId, String baseURI, S } + /** + * Print a warning message to the logs and UI if the build version does not match the XSD version. This is a best + * effort check, this method will never throw an exception. + */ + private void warnForMismatchedXsdVersion(String systemId) { + try { + Pattern versionPattern = Pattern.compile("(?:-pro-|-)(?[\\d.]*)\\.xsd"); + Matcher versionMatcher = versionPattern.matcher(systemId); + boolean found = versionMatcher.find(); + if (found) { + String buildVersion = LiquibaseUtil.getBuildVersion(); + if (!buildVersion.equals("DEV")) { + String xsdVersion = versionMatcher.group("version"); + if (!buildVersion.startsWith(xsdVersion)) { + String msg = "WARNING: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com"; + Scope.getCurrentScope().getLog(getClass()).warning(msg); + Scope.getCurrentScope().getUI().sendMessage(msg); + } + } + } + } catch (Exception e) { + Scope.getCurrentScope().getLog(getClass()).fine("Failed to compare XSD version with build version.", e); + } + } + /** * ResourceAccessor to use if the standard one does not have the XSD files in it. * Returns a ClassLoaderResourceAccessor that checks the system classloader which should include the liquibase.jar. diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy index 222bc5941d1..6839969c199 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy @@ -1,8 +1,11 @@ package liquibase.parser.core.xml import liquibase.GlobalConfiguration +import liquibase.LiquibaseTest import liquibase.Scope import liquibase.resource.FileSystemResourceAccessor +import liquibase.util.LiquibaseUtil +import org.xml.sax.InputSource import spock.lang.Specification import spock.lang.Unroll @@ -25,6 +28,45 @@ class LiquibaseEntityResolverTest extends Specification { ] } + @Unroll + def "warning message for mismatched xsd and build versions #systemId /// #buildVersion"() { + given: + def uiService = new LiquibaseTest.TestConsoleUIService() + // Save these props for later + def originalProperties = LiquibaseUtil.liquibaseBuildProperties + LiquibaseUtil.liquibaseBuildProperties = new Properties() + LiquibaseUtil.liquibaseBuildProperties.put("build.version", buildVersion) + + expect: + Scope.child([ + (Scope.Attr.ui.name()) : uiService + ], { + new LiquibaseEntityResolver().resolveEntity(null, null, null, systemId) + } as Scope.ScopedRunnerWithReturn) != null + + // This is an ugly assertion line, it is essentially saying, either we expect the message, so make sure it's there + // or we expect no message, so make sure there are no messages. + ((expectedWarningMessage && uiService.getMessages().contains("WARNING: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com")) + || (!expectedWarningMessage && uiService.getMessages().isEmpty())) + + cleanup: + // Set the build properties back to what they were before the test. + LiquibaseUtil.liquibaseBuildProperties = originalProperties + + where: + buildVersion | systemId | expectedWarningMessage + "3.1.0" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd" | false + "3.1.1" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd" | false + "4.12.0" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd" | true + "4.12.0" | "https://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd" | true + "4.12.0" | "http://www.liquibase.org/xml/ns/migrator/dbchangelog-3.1.xsd" | true + "4.12.0" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-next.xsd" | false + "4.12.0" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd" | false + "4.12.0" | "http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd" | false + "4.12.0" | "/liquibase/banner.txt" | false + "4.12.0" | "http://liquibase/banner.txt" | false + } + @Unroll def "resolveEntity finds packaged files correctly even if the configured resourceAccessor doesn't have it"() { expect: From 94db4233d5597eefed5a66effd5fef036ecf96b8 Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Thu, 30 Jun 2022 12:04:35 -0500 Subject: [PATCH 2/5] change warning level to info and only display on validate command --- .../src/main/java/liquibase/Liquibase.java | 16 +++++++++++++- .../core/xml/LiquibaseEntityResolver.java | 22 ++++++++++++++++--- .../core/xml/XMLChangeLogSAXParser.java | 8 +++++++ .../xml/LiquibaseEntityResolverTest.groovy | 6 +++-- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/Liquibase.java b/liquibase-core/src/main/java/liquibase/Liquibase.java index a34ce05e540..66606b789bd 100644 --- a/liquibase-core/src/main/java/liquibase/Liquibase.java +++ b/liquibase-core/src/main/java/liquibase/Liquibase.java @@ -33,6 +33,7 @@ import liquibase.logging.core.CompositeLogService; import liquibase.parser.ChangeLogParser; import liquibase.parser.ChangeLogParserFactory; +import liquibase.parser.core.xml.XMLChangeLogSAXParser; import liquibase.resource.InputStreamList; import liquibase.resource.ResourceAccessor; import liquibase.serializer.ChangeLogSerializer; @@ -364,8 +365,21 @@ public Connection getConnection(DatabaseChangeLog changeLog) throws LiquibaseHub public DatabaseChangeLog getDatabaseChangeLog() throws LiquibaseException { + return getDatabaseChangeLog(false); + } + + /** + * @param shouldWarnOnMismatchedXsdVersion When set to true, a warning will be printed to the console if the XSD + * version used does not match the version of Liquibase. If "latest" is used + * as the XSD version, no warning is printed. If the changelog is not xml + * format, no warning is printed. + */ + public DatabaseChangeLog getDatabaseChangeLog(boolean shouldWarnOnMismatchedXsdVersion) throws LiquibaseException { if (databaseChangeLog == null && changeLogFile != null) { ChangeLogParser parser = ChangeLogParserFactory.getInstance().getParser(changeLogFile, resourceAccessor); + if (parser instanceof XMLChangeLogSAXParser) { + ((XMLChangeLogSAXParser) parser).setShouldWarnOnMismatchedXsdVersion(shouldWarnOnMismatchedXsdVersion); + } databaseChangeLog = parser.parse(changeLogFile, changeLogParameters, resourceAccessor); } @@ -2278,7 +2292,7 @@ public DiffResult diff(Database referenceDatabase, Database targetDatabase, Comp */ public void validate() throws LiquibaseException { - DatabaseChangeLog changeLog = getDatabaseChangeLog(); + DatabaseChangeLog changeLog = getDatabaseChangeLog(true); changeLog.validate(database); } diff --git a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java index cca0ec2a9e1..89037b8cadb 100644 --- a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java +++ b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java @@ -23,6 +23,11 @@ public class LiquibaseEntityResolver implements EntityResolver2 { private static ClassLoaderResourceAccessor fallbackResourceAccessor; + private boolean shouldWarnOnMismatchedXsdVersion = false; + /** + * The warning message should only be printed once. + */ + private static boolean hasWarnedAboutMismatchedXsdVersion = false; @Override @java.lang.SuppressWarnings("squid:S2095") @@ -40,7 +45,9 @@ public InputSource resolveEntity(String name, String publicId, String baseURI, S .replace("http://www.liquibase.org/xml/ns/migrator/", "http://www.liquibase.org/xml/ns/dbchangelog/") .replaceFirst("https?://", ""); - warnForMismatchedXsdVersion(systemId); + if (shouldWarnOnMismatchedXsdVersion && !hasWarnedAboutMismatchedXsdVersion) { + warnForMismatchedXsdVersion(systemId); + } ResourceAccessor resourceAccessor = Scope.getCurrentScope().getResourceAccessor(); InputStreamList streams = resourceAccessor.openStreams(null, path); @@ -89,8 +96,9 @@ private void warnForMismatchedXsdVersion(String systemId) { if (!buildVersion.equals("DEV")) { String xsdVersion = versionMatcher.group("version"); if (!buildVersion.startsWith(xsdVersion)) { - String msg = "WARNING: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com"; - Scope.getCurrentScope().getLog(getClass()).warning(msg); + hasWarnedAboutMismatchedXsdVersion = true; + String msg = "INFO: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com"; + Scope.getCurrentScope().getLog(getClass()).info(msg); Scope.getCurrentScope().getUI().sendMessage(msg); } } @@ -121,4 +129,12 @@ public InputSource resolveEntity(String publicId, String systemId) throws SAXExc Scope.getCurrentScope().getLog(getClass()).warning("The current XML parser does not seems to not support EntityResolver2. External entities may not be correctly loaded"); return resolveEntity(null, publicId, null, systemId); } + + /** + * When set to true, a warning will be printed to the console if the XSD version used does not match the version + * of Liquibase. If "latest" is used as the XSD version, no warning is printed. + */ + public void setShouldWarnOnMismatchedXsdVersion(boolean shouldWarnOnMismatchedXsdVersion) { + this.shouldWarnOnMismatchedXsdVersion = shouldWarnOnMismatchedXsdVersion; + } } diff --git a/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java b/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java index 5c4e78467ca..e39e750f923 100644 --- a/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java +++ b/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java @@ -59,6 +59,14 @@ protected SAXParserFactory getSaxParserFactory() { return saxParserFactory; } + /** + * When set to true, a warning will be printed to the console if the XSD version used does not match the version + * of Liquibase. If "latest" is used as the XSD version, no warning is printed. + */ + public void setShouldWarnOnMismatchedXsdVersion(boolean shouldWarnOnMismatchedXsdVersion) { + resolver.setShouldWarnOnMismatchedXsdVersion(shouldWarnOnMismatchedXsdVersion); + } + @Override protected ParsedNode parseToNode(String physicalChangeLogLocation, ChangeLogParameters changeLogParameters, ResourceAccessor resourceAccessor) throws ChangeLogParseException { try (InputStream inputStream = resourceAccessor.openStream(null, physicalChangeLogLocation)) { diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy index 6839969c199..d47b6b201b5 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy @@ -36,17 +36,19 @@ class LiquibaseEntityResolverTest extends Specification { def originalProperties = LiquibaseUtil.liquibaseBuildProperties LiquibaseUtil.liquibaseBuildProperties = new Properties() LiquibaseUtil.liquibaseBuildProperties.put("build.version", buildVersion) + def er = new LiquibaseEntityResolver() + er.setShouldWarnOnMismatchedXsdVersion(true) expect: Scope.child([ (Scope.Attr.ui.name()) : uiService ], { - new LiquibaseEntityResolver().resolveEntity(null, null, null, systemId) + er.resolveEntity(null, null, null, systemId) } as Scope.ScopedRunnerWithReturn) != null // This is an ugly assertion line, it is essentially saying, either we expect the message, so make sure it's there // or we expect no message, so make sure there are no messages. - ((expectedWarningMessage && uiService.getMessages().contains("WARNING: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com")) + ((expectedWarningMessage && uiService.getMessages().contains("INFO: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com")) || (!expectedWarningMessage && uiService.getMessages().isEmpty())) cleanup: From d8c539b84635b6a424b379ca21296327982890a6 Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Thu, 30 Jun 2022 12:09:55 -0500 Subject: [PATCH 3/5] fix test --- .../liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy index d47b6b201b5..3126b8f572e 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy @@ -38,6 +38,7 @@ class LiquibaseEntityResolverTest extends Specification { LiquibaseUtil.liquibaseBuildProperties.put("build.version", buildVersion) def er = new LiquibaseEntityResolver() er.setShouldWarnOnMismatchedXsdVersion(true) + er.hasWarnedAboutMismatchedXsdVersion = false expect: Scope.child([ From 3b90f5655bd4079dbb20ff9ba7135c6c602e42dc Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Thu, 30 Jun 2022 12:25:58 -0500 Subject: [PATCH 4/5] make overloaded getDatabaseChangeLog method private --- liquibase-core/src/main/java/liquibase/Liquibase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liquibase-core/src/main/java/liquibase/Liquibase.java b/liquibase-core/src/main/java/liquibase/Liquibase.java index 66606b789bd..7d29b0ab54d 100644 --- a/liquibase-core/src/main/java/liquibase/Liquibase.java +++ b/liquibase-core/src/main/java/liquibase/Liquibase.java @@ -374,7 +374,7 @@ public DatabaseChangeLog getDatabaseChangeLog() throws LiquibaseException { * as the XSD version, no warning is printed. If the changelog is not xml * format, no warning is printed. */ - public DatabaseChangeLog getDatabaseChangeLog(boolean shouldWarnOnMismatchedXsdVersion) throws LiquibaseException { + private DatabaseChangeLog getDatabaseChangeLog(boolean shouldWarnOnMismatchedXsdVersion) throws LiquibaseException { if (databaseChangeLog == null && changeLogFile != null) { ChangeLogParser parser = ChangeLogParserFactory.getInstance().getParser(changeLogFile, resourceAccessor); if (parser instanceof XMLChangeLogSAXParser) { From 26f322ca6bf0d942227dbcc1f7e3b7a24a3af3a2 Mon Sep 17 00:00:00 2001 From: Steven Massaro Date: Thu, 30 Jun 2022 13:34:54 -0500 Subject: [PATCH 5/5] update info message --- .../java/liquibase/parser/core/xml/LiquibaseEntityResolver.java | 2 +- .../parser/core/xml/LiquibaseEntityResolverTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java index 89037b8cadb..b79279d2c09 100644 --- a/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java +++ b/liquibase-core/src/main/java/liquibase/parser/core/xml/LiquibaseEntityResolver.java @@ -97,7 +97,7 @@ private void warnForMismatchedXsdVersion(String systemId) { String xsdVersion = versionMatcher.group("version"); if (!buildVersion.startsWith(xsdVersion)) { hasWarnedAboutMismatchedXsdVersion = true; - String msg = "INFO: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com"; + String msg = "INFO: An older version of the XSD is specified in one or more changelog's header. This can lead to unexpected outcomes. If a specific XSD is not required, please replace all XSD version references with \"-latest\". Learn more at https://docs.liquibase.com"; Scope.getCurrentScope().getLog(getClass()).info(msg); Scope.getCurrentScope().getUI().sendMessage(msg); } diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy index 3126b8f572e..05f1e03959a 100644 --- a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/LiquibaseEntityResolverTest.groovy @@ -49,7 +49,7 @@ class LiquibaseEntityResolverTest extends Specification { // This is an ugly assertion line, it is essentially saying, either we expect the message, so make sure it's there // or we expect no message, so make sure there are no messages. - ((expectedWarningMessage && uiService.getMessages().contains("INFO: An older version of the XSD is specified in the changelog's header. This can lead to unexpected outcomes. Please update it to '" + buildVersion + "'. Learn more at https://docs.liquibase.com")) + ((expectedWarningMessage && uiService.getMessages().contains("INFO: An older version of the XSD is specified in one or more changelog's header. This can lead to unexpected outcomes. If a specific XSD is not required, please replace all XSD version references with \"-latest\". Learn more at https://docs.liquibase.com")) || (!expectedWarningMessage && uiService.getMessages().isEmpty())) cleanup: