Enhancing Main.java to help other programs call it.

Liquibase Development
1

Those seeem like good changes. Send a pull request and I can get them in.


I do want to improve the ability to integrate with Liquibase, the current Main method is definitely not ideal. My current direction was to try to pull a lot of the logic out of Main and have it be a much simpler wrapper around underlying APIs like the liquibase.Liquibase object and the new liquibase.configuration classes. Would you prefer to continue simply calling a static void main() method? Or would you rather have a more complete java API for integration?

Nathan

2

I’m one of the maintainers of Tim Berglund’s Groovy Liquibase DSL and the Gradle Liquibase plugin.  I’d like to propose some changes to the way that Main.java works that would make it easier for a plugin or other Java programs to interact with Liquibase.

 

The simplest, most decoupled way for something like the Gradle Liquibase plugin to invoke Liquibase is to simply call the main method as if it was being invoked on the command line, but certain behaviors in the main method are forcing me to make a copy of the Main.java file in the plugin code.  This means that improvements made in Liquibase are not available in the plugin until I manually update my copy of Main.java. 

 

I’d like to propose some changes to the way this class works.  I’m happy to contribute a pull request, but I want to discuss this first to make sure I’m heading in the right direction.  If we can make these changes, I (and other developers needing to call Liquibase from Java) can just call the Liquibase Main method.  Users of the Liquibase plugin could then take advantage of fixes in Liquibase by simply including the newer Liquibase version in their builds.

 

I’m currently looking to address the following issues:

 

  1. The main method currently uses System.exit when it is done.  This is a problem when invoking Liquibase from another program like a build tool because it causes the JVM to exit, and the build does not continue.

 

  1. When a change fails to execute, Liquibase currently logs this to the console with a stack trace 3 times, which can make it hard to find the original error.

 

  1. The main method doesn’t seem to have a way to take advantage of the new liquibaseCatalogName and liquibaseSchemaName properties.

 

  1. generateChangeLog is destructive, and there is no warning.

 

To do this I propose the following changes:

 

  1. Rename the main method to something else, like run.  Create a new main method that simply calls the run method in a try/catch block.  If the run method throws an exception, we can log thie exception and do a System.exit(-1).  If not, it would do a System.exit(0).  In the run method, System.exit(0), would be removed and the rest of the System.exit calls would be replaced with code to throw a new LiquibaseException.  This would preserve existing behavior for programs that call the main method now, but provide a new method that programs could call that would not cause the JVM to exit.  The one thing we’d need to determine is if the difference between the -1, -2, and -3 exit codes is significant.  If so, we’d need to throw different exceptions for each condition so the new main method could return the right thing.

 

  1. Main.applyDefaults() is called in a nested try/catch (line 131).  Removing that inner try/catch would eliminate one of the 3 stack traces.


3) The outer catch around main.applyDefaults() (line 157) coule be modified to remove the second of the 3 stack traces.  Instead of printing a stack trace, it could just get the exception’s cause if known, and log it instead of putting it out to system.err.

 

  1. We can add support for --liquibaseCatalogName and --liquibaseSchemaName parameters by adding liquibaseCatalogName and liquibaseSchemaName fields to the class, along with getters and setters, then changing the last 2 arguments to the CommandLineUtils.createDatabaseObject call on line 788 to use the new fields.


5) in doMigration, the generateChangeLog command call at line 807 is destructive.  I think we should add an if statement before the command execution to throw a LiquibaseException if the changeLogFile already exists.  This will prevent users from losing changes they may have made to their changeLog file.

 

Thanks for taking the time to read my little novel :-)  What do you think?

 

Steve Saliman

 

3

I’ll go ahead and do a pull request.  Would you prefer the change in 3.1.x or master?  3.1.x will make it available sooner, but it could lead to merge conflicts when you try to merge into master on the next release.

 

As for the API vs a main (or some other single) method makes more sense for my use case.  The API makes for simpler, richer integration for applications that want to use Liquibase programatically, but for a build tool plugin, I like the idea of a single method call because it would be more decoupled. 

 

My goal is for plugin users to have the ability to configure the plugin to use newer versions of Liquibase to take advantage of bug fixes without having to build a new version of the plugin itself.  If I’m just calling a single method and using reflection to find it, the plugin would simply work no matter what version of Liquibase was requested.

 

Steve

4

Forgot to reply, apparently.


I’d prefer the pull request to go into master. I’m trying to keep 3.1.x to primarily bug fixes and this seems larger than that. I’ve also not seen many major 3.1.x bugs and so the next release will probably be 3.2.0.


Good points on the main method usage. I am starting to shift to more of a command pattern that will be called by Main, ant tasks, etc. but the Main method will always be there as an easier integration point.


Nathan

5

I’ve done a pull request agains the master branch. Sorry it took so long, March was a really hectic month.

 

Steve

6

Thanks, I’ll take a look! Has been hectic for me as well so it may be a little bit until I get to it.

Nathan