XSD changes:

- remove non-root elements not allowing other root element than `databaseChangeLog` - added documentation for some elements - Fix: addNotNullConstraint@schemaName + @catalogName was missing - Non-reusable attributeGroups merged into the single elements they are used - New attributeGroups for common attributes: schemaNameAttribute - integerExp made based on xsd:long and non-negative - property@name restricted according to the documentation site - required string parameters made `nonEmptyString` to avoid SQL error for definitions like -`rowCount@expectedRows`, `setTableRemarks@remarks` made mandatory Code changes: - simple clear error message if the root element is other than `databaseChangeLog`: databaseChangeLog expected as root element - 'uniqueConstraint' was misspelled
b-gyula · Oct 16, 2024 · 68495fc · 68495fc
1 parent 946f4e4
commit 68495fc
Show file tree

Hide file tree

Showing 11 changed files with 1,155 additions and 1,061 deletions.
diff --git a/liquibase-standard/src/main/java/liquibase/change/core/CreateIndexChange.java b/liquibase-standard/src/main/java/liquibase/change/core/CreateIndexChange.java
@@ -169,7 +169,7 @@ public Boolean isUnique() {
      * <li>uniqueConstraint</li>
      * <li>none</li>
      * */
-    @DatabaseChangeProperty(description = "Index associations. Valid values: primaryKey, foreignKey, uniqueConstriant, none")
+    @DatabaseChangeProperty(description = "Index associations. Valid values: primaryKey, foreignKey, uniqueConstraint, none")
     public String getAssociatedWith() {
         return associatedWith;
     }

diff --git a/liquibase-standard/src/main/java/liquibase/change/core/DropIndexChange.java b/liquibase-standard/src/main/java/liquibase/change/core/DropIndexChange.java
@@ -63,7 +63,7 @@ public String getConfirmationMessage() {
     }
 
     @DatabaseChangeProperty(isChangeProperty = false,
-        description = "Index associations. Valid values: primaryKey, foreignKey, uniqueConstriant, none")
+        description = "Index associations. Valid values: primaryKey, foreignKey, uniqueConstraint, none")
     public String getAssociatedWith() {
         return associatedWith;
     }

diff --git a/liquibase-standard/src/main/java/liquibase/parser/ChangeLogParser.java b/liquibase-standard/src/main/java/liquibase/parser/ChangeLogParser.java
@@ -10,7 +10,7 @@
  * are able to parse different file formats (e.g. XML, YAML, JSON, ...)
  */
 public interface ChangeLogParser extends LiquibaseParser {
-
+    String DATABASE_CHANGE_LOG = "databaseChangeLog";
     /**
      * Parses a Liquibase database changelog and returns the parsed form as an object.
      * @param physicalChangeLogLocation the physical location of the changelog. The exact file formats and locations

diff --git a/liquibase-standard/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java b/liquibase-standard/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java
@@ -69,7 +69,10 @@ public void setShouldWarnOnMismatchedXsdVersion(boolean shouldWarnOnMismatchedXs
     }
 
     @Override
-    protected ParsedNode parseToNode(String physicalChangeLogLocation, ChangeLogParameters changeLogParameters, ResourceAccessor resourceAccessor) throws ChangeLogParseException {
+    protected ParsedNode parseToNode(String physicalChangeLogLocation,
+                                     ChangeLogParameters changeLogParameters,
+                                     ResourceAccessor resourceAccessor)
+          throws ChangeLogParseException {
         try {
             Resource resource = resourceAccessor.get(physicalChangeLogLocation);
             SAXParser parser = saxParserFactory.newSAXParser();
@@ -80,6 +83,7 @@ protected ParsedNode parseToNode(String physicalChangeLogLocation, ChangeLogPara
                     Scope.getCurrentScope().getLog(getClass()).fine("Cannot enable ACCESS_EXTERNAL_SCHEMA: " + e.getMessage(), e);
                 }
             }
+
             trySetSchemaLanguageProperty(parser);
 
             XMLReader xmlReader = parser.getXMLReader();
@@ -127,7 +131,12 @@ public void fatalError(SAXParseException exception) throws SAXException {
         } catch (IOException e) {
             throw new ChangeLogParseException("Error Reading Changelog File: " + e.getMessage(), e);
         } catch (SAXParseException e) {
-            throw new ChangeLogParseException("Error parsing line " + e.getLineNumber() + " column " + e.getColumnNumber() + " of " + physicalChangeLogLocation + ": " + e.getMessage(), e);
+            String errMsg =  e.getMessage();
+            if(e.getMessage().startsWith("cvc-elt.1.a")){
+                errMsg = '"' + DATABASE_CHANGE_LOG + "\" expected as root element";
+            }
+            throw new ChangeLogParseException("Error parsing line " + e.getLineNumber() + " column "
+                  + e.getColumnNumber() + " of " + physicalChangeLogLocation + ": " + errMsg, e);
         } catch (SAXException e) {
             Throwable parentCause = e.getException();
             while (parentCause != null) {

diff --git a/liquibase-standard/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java b/liquibase-standard/src/main/java/liquibase/parser/core/yaml/YamlChangeLogParser.java
@@ -25,7 +25,6 @@
 import java.util.*;
 
 public class YamlChangeLogParser extends YamlParser implements ChangeLogParser {
-    private static final String DATABASE_CHANGE_LOG = "databaseChangeLog";
 
     @Override
     public DatabaseChangeLog parse(String physicalChangeLogLocation, ChangeLogParameters changeLogParameters, ResourceAccessor resourceAccessor) throws ChangeLogParseException {

diff --git a/liquibase-standard/src/main/java/liquibase/precondition/CustomPreconditionWrapper.java b/liquibase-standard/src/main/java/liquibase/precondition/CustomPreconditionWrapper.java
@@ -111,7 +111,7 @@ public String getName() {
     }
 
     @Override
-    protected boolean shouldAutoLoad(ParsedNode node) {
+    public boolean shouldAutoLoad(ParsedNode node) {
         if ("params".equals(node.getName()) || "param".equals(node.getName())) {
             return false;
         }

diff --git a/...e-standard/src/main/resources/www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd b/...e-standard/src/main/resources/www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd
diff --git a/liquibase-standard/src/test/groovy/liquibase/changelog/DatabaseChangeLogTest.groovy b/liquibase-standard/src/test/groovy/liquibase/changelog/DatabaseChangeLogTest.groovy
@@ -1,6 +1,7 @@
 package liquibase.changelog
 
 import liquibase.ContextExpression
+import liquibase.GlobalConfiguration
 import liquibase.LabelExpression
 import liquibase.Labels
 import liquibase.Scope
@@ -9,11 +10,13 @@ import liquibase.change.core.RawSQLChange
 import liquibase.change.visitor.ChangeVisitor
 import liquibase.database.Database
 import liquibase.database.core.MockDatabase
+import liquibase.exception.ChangeLogParseException
 import liquibase.exception.SetupException
 import liquibase.exception.UnexpectedLiquibaseException
 import liquibase.logging.core.BufferedLogService
 import liquibase.parser.ChangeLogParserConfiguration
 import liquibase.parser.core.ParsedNode
+import liquibase.parser.core.xml.XMLChangeLogSAXParser
 import liquibase.precondition.core.OrPrecondition
 import liquibase.precondition.core.PreconditionContainer
 import liquibase.precondition.core.RunningAsPrecondition
@@ -29,12 +32,13 @@ import spock.lang.Unroll
 
 import java.nio.file.Paths
 import java.util.logging.Level
-import java.util.stream.Stream
+
+import static java.lang.Boolean.FALSE
+import static liquibase.parser.ChangeLogParser.DATABASE_CHANGE_LOG
 
 class DatabaseChangeLogTest extends Specification {
 
-    @Shared
-            resourceSupplier = new ResourceSupplier()
+    @Shared resourceSupplier = new ResourceSupplier()
     def test1Xml = '''<databaseChangeLog
         xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
@@ -172,6 +176,7 @@ create view sql_view as select * from sql_table;'''
         ((CreateTableChange) changeLogFromChildren.changeSets[1].changes[0]).tableName == "my_other_table"
         ((CreateTableChange) changeLogFromValue.changeSets[1].changes[0]).tableName == "my_other_table"
     }
+
     def "load handles removeChangeSetProperty"() {
         when:
         def children = [
@@ -762,7 +767,6 @@ http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbch
         result
     }
 
-
     def "warning message is logged when changelog include fails because file does not exist"() {
         when:
         def rootChangeLogPath = "com/example/root.xml"
@@ -1077,4 +1081,65 @@ http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbch
         "5.xml"         | 0
     }
 
+    @Unroll
+    def "#tag as root element"() {
+        when:
+        String content = """<$tag xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                                xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                                xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog 
+                                    www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd"/>
+        """
+        String rootChangeLogPath = "root.xml"
+        def resourceAccessor = new MockResourceAccessor([(rootChangeLogPath): content])
+
+        DatabaseChangeLog changeLog =
+           Scope.child([(GlobalConfiguration.SECURE_PARSING.key): FALSE], {
+               // Create a new parser instead of using the parserFactory to make sure SECURE_PARSING is used
+               new XMLChangeLogSAXParser().parse(rootChangeLogPath, new ChangeLogParameters(), resourceAccessor)
+           } as Scope.ScopedRunnerWithReturn<DatabaseChangeLog>)
+
+        then:
+        ChangeLogParseException e = thrown()
+        /Error parsing line 4 column 99 of root.xml: "$DATABASE_CHANGE_LOG" expected as root element/ == e.message
+        where:
+        tag << [
+            'addAutoIncrement', 'addColumn',
+            'addDefaultValue',  'addForeignKeyConstraint',
+            'addLookupTable',   'addNotNullConstraint',
+            'addPrimaryKey',    'addUniqueConstraint',
+            'alterSequence',    'and',
+            'changeLogPropertyDefined',
+            'changeSetExecuted','column',
+            'columnExists',     'comment',
+            'constraints',      'createIndex',
+            'createProcedure',  'createSequence',
+            'createTable',      'createView',
+            'customChange',     'customPrecondition',
+            'dbms',             'delete',
+            'dropAllForeignKeyConstraints','dropColumn',
+            'dropDefaultValue', 'dropForeignKeyConstraint',
+            'dropIndex',        'dropNotNullConstraint',
+            'dropPrimaryKey',   'dropProcedure',
+            'dropSequence'  ,   'dropTable',
+            'dropUniqueConstraint','dropView',
+            'empty',            'executeCommand',
+            'expectedQuotingStrategy','foreignKeyConstraintExists',
+            'include',          'includeAll',
+            'insert',           'loadData',
+            'loadUpdateData',   'mergeColumns',
+            'modifyDataType',   'not',
+            'or',   'output',   'param',
+            'primaryKeyExists', 'preConditions',
+            'renameSequence',   'renameTable',
+            'renameView',       'rollback',
+            'rowCount',         'runningAs',
+            'sequenceExists',   'setColumnRemarks',
+            'setTableRemarks',  'sql',
+            'sqlCheck',         'sqlFile',
+            'stop',             'tableExists',
+            'tableIsEmpty',     'tagDatabase',
+            'uniqueConstraintExists', 'update',
+            'viewExists',       'whereParams'
+            ]
+    }
 }
diff --git a/...dard/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParser_RealFile_Test.groovy b/...dard/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParser_RealFile_Test.groovy
@@ -10,12 +10,14 @@ import com.example.liquibase.change.KeyColumnConfig
 import com.example.liquibase.change.PrimaryKeyConfig
 import com.example.liquibase.change.UniqueConstraintConfig
 import liquibase.Contexts
+import liquibase.GlobalConfiguration
 import liquibase.Scope
 import liquibase.change.Change
 import liquibase.change.ChangeFactory
 import liquibase.change.CheckSum
 import liquibase.change.core.AddColumnChange
 import liquibase.change.core.CreateIndexChange
+import liquibase.change.core.CreateProcedureChange
 import liquibase.change.core.CreateSequenceChange
 import liquibase.change.core.CreateTableChange
 import liquibase.change.core.CreateViewChange
@@ -45,6 +47,7 @@ import liquibase.precondition.core.OrPrecondition
 import liquibase.precondition.core.PreconditionContainer
 import liquibase.precondition.core.PrimaryKeyExistsPrecondition
 import liquibase.precondition.core.RunningAsPrecondition
+import liquibase.resource.ClassLoaderResourceAccessor
 import liquibase.sdk.supplier.resource.ResourceSupplier
 import liquibase.sql.visitor.AppendSqlVisitor
 import liquibase.sql.visitor.ReplaceSqlVisitor
@@ -54,6 +57,7 @@ import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
 
+import static java.lang.Boolean.FALSE
 import static org.hamcrest.Matchers.containsInAnyOrder
 import static spock.util.matcher.HamcrestSupport.that
 
@@ -814,4 +818,36 @@ class XMLChangeLogSAXParser_RealFile_Test extends Specification {
         Scope.getCurrentScope().getSingleton(ChangeFactory.class).unregister("primitiveChange")
     }
 
+    def "parse comments from comments.xml"() throws Exception {
+        def path = "liquibase/parser/core/xml/comments.xml"
+        when:
+        DatabaseChangeLog changeLog =
+                Scope.child([(GlobalConfiguration.SECURE_PARSING.key): FALSE], {
+                    new XMLChangeLogSAXParser().parse(path, new ChangeLogParameters(), new ClassLoaderResourceAccessor())
+                } as Scope.ScopedRunnerWithReturn<DatabaseChangeLog>)
+        def changeSet = changeLog.changeSets[0]
+
+        then: 'changeset comments are parsed'
+        changeSet.comments == 'changeSet comment 1\nchangeSet comment 2'
+        changeSet.changes.size() == 3
+        changeSet.validCheckSums.contains( CheckSum.parse('2:aaafdfadsf'))
+        changeSet.validCheckSums.contains( CheckSum.parse('1:any'))
+        changeSet.validCheckSums.size() == 2
+
+        and: 'sql 1 parsed'
+        def sql1 = changeSet.changes[0] as RawSQLChange
+        sql1.comment == 'sql comment'
+        sql1.sql == 'SELECT COUNT(*) FROM DATABASECHANGELOG'
+
+        and: 'sql2 parsed'
+        def sql2 = changeSet.changes[1] as RawSQLChange
+        sql2.comment == null
+        sql2.sql == 'SELECT COUNT(*) FROM DATABASECHANGELOGLOCK'
+
+        and: 'create procedure parsed'
+        def cp = changeSet.changes[2] as CreateProcedureChange
+        // Fails cp.comments == 'createProcedure comment'
+        cp.procedureText == 'SELECT 1 FROM DATABASECHANGELOG'
+
+    }
 }
diff --git a/liquibase-standard/src/test/resources/liquibase/change/ChangeDefinitionTest.yaml b/liquibase-standard/src/test/resources/liquibase/change/ChangeDefinitionTest.yaml
@@ -330,7 +330,7 @@ alterSequence: |
 
 createIndex: |
   associatedWith string 
-    Description: Index associations. Valid values: primaryKey, foreignKey, uniqueConstriant, none
+    Description: Index associations. Valid values: primaryKey, foreignKey, uniqueConstraint, none
     Supported: all
   catalogName string (since 3.0)
     Description: Name of the database catalog

diff --git a/liquibase-standard/src/test/resources/liquibase/parser/core/xml/comments.xml b/liquibase-standard/src/test/resources/liquibase/parser/core/xml/comments.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+						 xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+						 www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
+	<changeSet id="1" author="">
+		<validCheckSum>1:any
+			<comment>validCheckSum comment</comment>
+		</validCheckSum>
+		<validCheckSum>2:aaafdfadsf</validCheckSum>
+		<comment>changeSet comment 1</comment>
+		<sql><comment>sql comment</comment>
+			SELECT COUNT(*) FROM DATABASECHANGELOG</sql>
+		<comment>changeSet comment 2</comment>
+		<sql>SELECT COUNT(*) FROM DATABASECHANGELOGLOCK</sql>
+		<createProcedure>
+			<comment>createProcedure comment</comment>
+			SELECT 1 FROM DATABASECHANGELOG
+		</createProcedure>
+	</changeSet>
+</databaseChangeLog>