Prevent silent failures when two "databaseChangeLog" tags are present #1826
Conversation
|
Hi @danielthegray Thanks for the improvement in the use of Liquibase. It is much appreciated. Here's what happens next:
The PR will be prioritized according to our internal development and testing capacity. We’ll let you know when it’s ready to move to the next step or if any changes are needed. |
|
@danielthegray SnakeYAML has a property to prohibit the same keys |
52dce37 to
da61869
Compare
da61869 to
fd33644
Compare
fd33644 to
8af7e84
Compare
Thank you @asomov for the great tip, it greatly simplifies the code! I've pushed a new revision of the branch with conflicts resolved, but the "FOSSA License Compliance and Security Check" is failing. Does something need to be added to the branch code? |
|
@daniel-gray-jemmic currently fossa is failing for contributed PR's, don't worry about it. |
8af7e84 to
83c3977
Compare
...standard/src/test/groovy/liquibase/parser/core/yaml/YamlChangeLogParser_RealFile_Test.groovy
Show resolved
Hide resolved
83c3977 to
b161673
Compare
b161673 to
64fd85e
Compare
64fd85e to
bb26acb
Compare
If someone makes a mistake and copy/pastes a databaseChangeLog block in, and makes the file have two tags, the SnakeYAML parser silently covers up the first instance with the second one, which will cause the first block of changes to be silently omitted. This can be very problematic and cause hard-to-debug errors, which is why I have added a small FilterInputStream interceptor to check the stream as it is fed into the YAML parser, for a duplicate tag. Doing it with the FilterInputStream like this prevents the efficiency losses of opening the file twice.
bb26acb to
c7eacc6
Compare
… with yaml format changelogs.
|
I just wanted to let you know today I commented out the line you added on the If you have any ideas or would like to work on it, please feel free to assign it to yourself or ask any questions/concerns you might have. Thank you and apologies for this, |
|
Hi, I understand. The first commit I had on this issue was ONLY detecting the duplicate databaseChangeLog, and I changed it to the duplicate flag because of a suggestion in a comment on this thread. Should we try to revert to the previous version instead? |
@danielthegray I think this would be a better approach, thanks! |
Environment
Liquibase Version: all
Liquibase Integration & Version: all
Liquibase Extension(s) & Version: all
Database Vendor & Version: all
Operating System Type & Version: all
Pull Request Type
Description
If someone makes a mistake and copy/pastes a databaseChangeLog block in,
and makes the file have two tags, the SnakeYAML parser silently covers
up the first instance with the second one, which will cause the first
block of changes to be silently omitted.
This can be very problematic and cause hard-to-debug errors, which is
why I have added a small FilterInputStream interceptor to check the
stream as it is fed into the YAML parser, for a duplicate tag. Doing it
with the FilterInputStream like this prevents the efficiency losses of
opening the file twice.
Steps To Reproduce
You can look at the added test; essentially, just create a changelog file which has two "databaseChangeLog" tags.
Actual Behavior
It will silently to the wrong thing (i.e. skip over the first group of changesets).
Expected/Desired Behavior
An error should be thrown to highlight the fact that there are two databaseChangeLog tags/keys.
Fast Track PR Acceptance Checklist:
Need Help?
Come chat with us on our discord channel