liquibase: Liquibase lib upgrade to 4.21.1 breaking rollback working earlier

Anyone would be able to do create it so I can fast track to the next release probabily next week?

Apologies if it oversteps, but here is a PR suggestion: #4591

This looks good. Having the exact same issue.

For us it is on change sets where we are using properties from liquibase.properties.

Oh, of course! I was uselessly searching for “4406” on the release notes 🤦 Thank you very much!

Hello @blaghed ! You mean the changes on this PR -> https://github.com/liquibase/liquibase/pull/4591 ? They were included in Liquibase 4.24.0 😃

@blaghed we are changing our build process and in the last week or so we were not able to run our full test suite against some PR’s, and we need all steps to pass before merging to master . Hopefully it will be fixed still this week so we can get it merged for next release.

Thanks @rdhzl and @blaghed ! The code I added above is mostly a copy from what we did on Liquibase class when we refactored rollback command. So we can create a PR to add the missing CHANGELOG_PARAMETERS . Anyone would be able to do create it so I can fast track to the next release probabily next week?

About Liquibase class @rdhzl , it will stay deprecated until Liquibase 5 is released. That said, we have no plans for 5 so far, and Liquibase class may even be refactored again for it, but there is a chance it goes away and the way forward is the Command framework - and if Liquibase class remains it will be just wrappers for commands. So Scope.child and CommandScope are the present and the future .

Hi @filipelautert ,

We are also facing this issue, as far as I can understand it. A quick browse seems to show that this may have been accidentally caused during a refactor between 4.20.0 and 4.21.0, and then further buried with a much larger refactor of the project’s layout during 4.22.0.

With the current latest version, there is a new (much tidier) approach to run liquibase, but it does require setting up the execution correctly, as we can see, for example, on the update command: https://github.com/liquibase/liquibase/blob/v4.23.0/liquibase-standard/src/main/java/liquibase/Liquibase.java#L311

    public void update(Contexts contexts, LabelExpression labelExpression, Writer output, boolean checkLiquibaseTables)
            throws LiquibaseException {
        runInScope(() -> {
            CommandScope updateCommand = new CommandScope(UpdateSqlCommandStep.COMMAND_NAME);
            updateCommand.addArgumentValue(DbUrlConnectionCommandStep.DATABASE_ARG, getDatabase());
            updateCommand.addArgumentValue(UpdateSqlCommandStep.CHANGELOG_FILE_ARG, changeLogFile);
            updateCommand.addArgumentValue(UpdateSqlCommandStep.CONTEXTS_ARG, contexts != null ? contexts.toString() : null);
            updateCommand.addArgumentValue(UpdateSqlCommandStep.LABEL_FILTER_ARG, labelExpression != null ? labelExpression.getOriginalString() : null);
            updateCommand.addArgumentValue(ChangeExecListenerCommandStep.CHANGE_EXEC_LISTENER_ARG, changeExecListener);
            updateCommand.addArgumentValue(DatabaseChangelogCommandStep.CHANGELOG_PARAMETERS, changeLogParameters); // highlighting this part
            updateCommand.setOutput(new WriterOutputStream(output, GlobalConfiguration.OUTPUT_FILE_ENCODING.getCurrentValue()));
            updateCommand.execute();
        });
    }

On the rollback command, you may note that the “DatabaseChangelogCommandStep.CHANGELOG_PARAMETERS” parameters are not passed in at all: https://github.com/liquibase/liquibase/blob/v4.23.0/liquibase-standard/src/main/java/liquibase/Liquibase.java#L488-L500

    public void rollback(int changesToRollback, String rollbackScript, Contexts contexts,
                         LabelExpression labelExpression, Writer output) throws LiquibaseException {
        new CommandScope(RollbackCountSqlCommandStep.COMMAND_NAME)
                .addArgumentValue(DbUrlConnectionCommandStep.DATABASE_ARG, Liquibase.this.getDatabase())
                .addArgumentValue(DatabaseChangelogCommandStep.CHANGELOG_FILE_ARG, changeLogFile)
                .addArgumentValue(DatabaseChangelogCommandStep.CONTEXTS_ARG, (contexts != null? contexts.toString() : null))
                .addArgumentValue(DatabaseChangelogCommandStep.LABEL_FILTER_ARG, (labelExpression != null ? labelExpression.getOriginalString() : null))
                .addArgumentValue(ChangeExecListenerCommandStep.CHANGE_EXEC_LISTENER_ARG, changeExecListener)
                .addArgumentValue(RollbackCountCommandStep.COUNT_ARG, changesToRollback)
                .addArgumentValue(AbstractRollbackCommandStep.ROLLBACK_SCRIPT_ARG, rollbackScript)
                .setOutput(new WriterOutputStream(output, GlobalConfiguration.OUTPUT_FILE_ENCODING.getCurrentValue()))
                .execute();
    }

There are plenty of other methods in that class making use of “addArgumentValue” that also don’t set these parameters, so please be sure to check those as well, but I believe that those are likely correct. Only the several “rollback” operations are, imho, mistakenly missing that.

This seems like an honest mistake happening due to the refactors, so likely it just needs a small patch for having a quick win. I don’t mind contributing it myself, but I get the feeling I will trip all over your contribution guidelines. If you simply don’t have time for it yourself, however, I will be happy to do so, so just let me know.

Hello @rdhzl ! would you be able to test it again with Liquibase 4.23.0 ? We added some fixes related to those methods in that version.