Inconsistent resolution of ChangeLogParameters in liquibase.changelog.ChangeLogParameters#findParameter

Description

This is related to CORE-2309, CORE-2362 and CORE-3293.

The current implementation (up to v3.8.0 and 3.9.0-SNAPSHOT) of ChangeLogParameters#findParameter(..) does not even check if a parameter is global or local.

  1. If there is only one parameter matching the name in the parameter list, at the time of any given expression expansion, that value gets returned no matter what. -> This effectively applies that parameter like it was global even if it is global=false including in <include file="" />.

  2. If more than one matching parameter is found, the last one declared in the same changeset file is applied. -> This time the effect is always, as if that parameter was declared global=false, even if that is not the case. The value is then not applied to <include file="" />.

No matter what exactly liquibase considers global=false to mean, this current implementation makes no sense. It also does not satisfy the reason why the global attribute was introduced in the first place as stated in https://forum.liquibase.org/#Topic/49382000001251007 which let to CORE-2309.


I personally consider <include file="" /> to mean the same as if the changeSets in that included file where inserted directly in the changeset file at the exact same position, the <include file="" /> is located at.
If that assumption is correct, then ChangeLogParameters declared as global=false should always also be applied to all included files (even recursively), which are declared in the same changeset file after the declaration of said ChangeLogParameter.


My proposal is therefore:

  • if any parameters were found

    • look for the first global parameter and return that (this prevents overwriting - a decision made by liquibase in the past)

    • if none of the found parameters are global (all of them are local)

      • look for the first parameter belonging to the current changeLog

      • or if that fails look for the first parameter belonging to the parent of the current changeLog

      • or if that fails ... (you get the picture) -> so in effect the closest ancestor of the changeLog

      • stop if there are no more ancestors (we reached the root changelog file)

  • if no matching parameter was found by now, return null

    • this should probably result in an exception at the expression espansion, since it likely means someone forgot to place a parameter declaration somewhere

I created a pull request for my implementation of the above algorithm.


My solution allows to write changeset "templates" correlating to abstract super classes in JAVA.
e.g.:

  • changelogs

    • 0016-create-product-table.xml (<property name="table.name" value="product" global="false" /> , include 0002-create-versioned-data-template, add more columns based on parameters)

    • 0020-create-credential-table.xml (table.name=credential, include 0001-create-identifiable-data-template, add more columns based on parameters)

  • templates

    • 0001-create-identifiable-data-template.xml (create table with id based on parameters)

    • 0002-create-versioned-data-template.xml (include 0001-create-identifiable-data-template, add more columns (version, creator ...) based on parameters)

  • db.changelog-master.xml (includes files in changelogs)


However even that solution does not produce a fully consistent application of the parameters. The reason is, that liquibase currently starts expanding expressions before all changesets and therefore all ChangeLogParameters have been read. That was fine as long as parameters are only allowed to be declared once and may not be overwritten. It still only worked if those global ChangeLogParameters were declared before their first use, so they were not truly global then either, but it was close enough.

My solution would have a very similar limitation and would require global ChangeLogParameters first.
If a ChangeLogParameter would be declared both global and local at different points in the changesets, then the local values would be applied until the global value has been parsed, even though the global value should have superseded the local declaration. After the first global value has been read all other occurrences will now use the global version, even if there is a local version in the current changeset. On the other hand a mix of global and local versions of a parameter probably does not make much sense anyway.

If that should ever be fully resolved, then all changesets and parameters need to be read without expanding expressions. Only after everything has been parsed should expressions be expanded. However, I lack the oversight to say if that has other negative side effects. That should be checked first.

Environment

any

Status

Assignee

Unassigned

Reporter

Holly Schöne

Labels

None

Components

Affects versions

Priority

Major
Configure