liquibase / liquibase-groovy-dsl

The official Groovy DSL for Liquibase
Other
83 stars 34 forks source link

Property support in DSL #35

Closed sgilhooly closed 6 years ago

sgilhooly commented 6 years ago

I'm trying to define a column in the DSL which has a defaultValueCompleted argument which will vary based on the SQL dialect chosen. In XML/Yaml change set definition this is handled with a property based on the dbms matching:

    <databaseChangeLog . . .>
        <property name="example" value="mysql-specific-statement" dbms="mysql" />
        <property name="example" value="some-other-specific-statement" dbms="h2,postgres" />
        . . .
    </databaseChangeLog>

When I convert this to Groovy DSL:

   databaseChangeLog {
       property(name: 'example', value: 'mysql-specific-statement', dbms: 'mysql')
       property(name: 'example', value: 'some-other-specific-statement', dbms: 'h2,postgres')
       // etc...
    }

Parsing fails to recognize the property entry: liquibase.exception.ChangeLogParseException: Unrecognized root element property

Ultimately, I don't need the property thing to work if there was some way to determine the dialect in use in the groovy-ness of this. For example, something like:

  databaseChangeLog {
      def myStatement
      if (someGlobal.dbms == 'mysql') {
          myStatement = 'mysql-specific-statement'
      } else {
          myStatement = 'generic-statement-that works-for-other-db-types'
      }
  }

So it feels like the support for the property element is either missing or perhaps the groovy substitution for it is just not documented (anywhere I looked).

stevesaliman commented 6 years ago

I'm unable to reproduce this issue using the Gradle plugin and version 2.0.1 of the DSL. I added the two property statements you had to one of my changelogs, and it worked as I expected. Do you have a sample project that exhibits this behavior?

sgilhooly commented 6 years ago

I created an MCVE here: https://github.com/sgilhooly/lb-prop-mcve

When I run a gradle build on this project (using gradle v4.9) I get the error:

> Task :generateModel FAILED
Starting in-memory DB...
Running liquibase change sets...

FAILURE: Build failed with an exception.

* Where:
Build file '/Users/sgilhooly/Code/lb-prop-mcve/build.gradle' line: 50

* What went wrong:
Execution failed for task ':generateModel'.
> liquibase.exception.ChangeLogParseException: Unrecognized root element property

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 0s
stevesaliman commented 6 years ago

Thank you for the sample. I do indeed get the error when I run your example. The first thing I noticed is that you're not using the Liquibase Gradle plugin to run Liquibase. Is there a reason you can't use it in your environment? There may be a subtle difference in the way Liquibase initializes when calling it directly vs. going through it's main method like the plugin does.

sgilhooly commented 6 years ago

I'm not using the plugin since the plugin doesn't work for my use case. The full build starts an in-memory H2 database, runs liquibase to seed a schema, and then runs the JOOQ generator on that DB to generate the Java access classes. So I need the H2 in-memory database to stick around after liquibase is done with it (which the plugin doesn't do).

stevesaliman commented 6 years ago

I spent some time in the debugger trying to figure out why the property method wasn't being called in the DSL. I finally figured it out: the dbms argument is wrong. It needs to be dbms: 'mysql' instead of dbms='mysql' The code snippet in the original issue is correct, but the changelog in the project has an equals sign.

Try changing the equals to a colon and see if that fixes the issue for you.

sgilhooly commented 6 years ago

😳 Ahhhh. Good find. Clearly, you can close this issue as "boneheaded user error." Thanks for the assist and sorry to steal your time.

stevesaliman commented 6 years ago

No worries. Sometimes it just takes another set of eyes on a problem. Glad it's working now.