API inconsistency - Liquibase creation file vs. DatabaseChangeLog

Betlista commented 3 years ago

Environment

Liquibase Version: 4.2.2

Liquibase Integration & Version: Spring Boot

Liquibase Extension(s) & Version: N/A

Database Vendor & Version: HSQLDB

Operating System Type & Version: Win 10

Description

There is an inconsistency, while this works:

File file = changelogResource.getFile();
File parentFile = file.getParentFile();
Liquibase liquibase = new Liquibase(file.getName(), new FileSystemResourceAccessor(parentFile), getDatabase());
liquibase.update((String)null);

this does not work:

File file = changelogResource.getFile();
File parentFile = file.getParentFile();
DatabaseChangeLog databaseChangeLog = new DatabaseChangeLog(file.getName());
Liquibase liquibase = new Liquibase(databaseChangeLog, new FileSystemResourceAccessor(parentFile), getDatabase());
liquibase.update((String)null);

Steps To Reproduce

code above

Actual Behavior

In second scenario, the change sets are not loaded.

Expected/Desired Behavior

It it very similar code, so I'd expect both to work same way = change sets are loaded

Screenshots (if appropriate)

N/A

Additional Context

N/A

Betlista commented 3 years ago

The problem seems to be in getDatabaseChangeLog(), which is called from update(Contexts contexts, LabelExpression labelExpression, boolean checkLiquibaseTables):

    public DatabaseChangeLog getDatabaseChangeLog() throws LiquibaseException {
        if (this.databaseChangeLog == null && this.changeLogFile != null) {
            ChangeLogParser parser = ChangeLogParserFactory.getInstance().getParser(this.changeLogFile, this.resourceAccessor);
            this.databaseChangeLog = parser.parse(this.changeLogFile, this.changeLogParameters, this.resourceAccessor);
        }

        return this.databaseChangeLog;
    }

...where if condition really says "do not do the parsing if databaseChangeLog was provided", which seems to be wrong to me still.

While we need changeLogFile to get parsed databaseChangeLog, offering public Liquibase(DatabaseChangeLog changeLog, ResourceAccessor resourceAccessor, Database database) seems wrong as well as calling ChangeLogParserFactory.getInstance().getParser(...) following by parser.parse(...) with same arguments...

I'd suggest marking public Liquibase(DatabaseChangeLog changeLog, ResourceAccessor resourceAccessor, Database database) deprecated (it's not referenced anyway) and probably remove it in some later version.

molivasdat commented 3 years ago

Hi @Betlista Thanks for writing up this issue. We will add it to the list of issues we are processing.

liquibase / liquibase