Property substitution support

[hoxu]

Is there anything similar to Liquibase's property substitution and Flyway's placeholders?

• https://docs.liquibase.com/concepts/basic/changelog-property-substitution.html
• https://flywaydb.org/documentation/configuration/placeholder

[yogthos]

There isn't any support for property substitution at the moment, but I'd be open to adding that as a feature.

[yogthos]

I just added support on a branch here. Could you take a look to see if that matches what you were thinking, and if it looks good I could push out a new version with the feature.

[hoxu]

That was fast, thanks!

I read the README.md diff. I think taking properties only from the environment variables is not flexible enough, although I do think either environment or System properties are one good way to allow providing these.

For comparison, Liquibase allows:

1. Passed as a attribute to your Liquibase runner. See the Ant, Maven, or Servlet Listener documentation for more information on how to pass them.
2. As a JVM system property.
3. As an environment variable.
4. As a CLI attribute if executed from the command line.
5. In the liquibase.properties file if used or executed from the command line. See the Creating and configuring a liquibase.properties file for more information.
6. In the parameters block (property element of the DATABASECHANGELOG table itself.

Flyway:

1. Via environment variables. FLYWAY_PLACEHOLDERS_MYPLACEHOLDER=value
2. Via configuration parameters. flyway.placeholders.myplaceholder=value
3. Via the api. .placeholders(Map.of("myplaceholder", "value"))

The usual usecase with property substitution that I have seen is having things like

GRANT SELECT,INSERT ON example_table TO ${database.user};

Then you can run the same migrations for different users.

For production providing this as an environment variable or a JVM system property is fine.

For development being able to run the migrations multiple times with different properties is nice (eg. separate development and integration test databases), so only ENV variables are not enough.

If you want to keep things really simple with Migratus, I think the best would be to just allow providing a map, like so:

:custom-properties {:database {:user "username"}}

Now ${database.user} would be substituted with username.

This way if people want to dig the properties from environment, JVM properties, or where ever, they can write explicit code.

In any case, please allow providing an explicit substitution map!

[yogthos]

Thanks for the feedback, I added the option to specify :custom-properties as you suggested:

custom env property names will now be specified using the :custom-env-properties key and a map of custom properties can be assigned to the :custom-properties key:

{:store                 :database
 :inject-properties?    true
 :custom-env-properties ["database.table"]
 :custom-properties     {:database {:user "bob"}}}

The database.table key will replace ${database.table} in the template with the value found in the environment, while {:database {:user "bob"}} will replace ${database.user} with "bob".

GRANT SELECT,INSERT ON ${database.table} TO ${database.user};

let me know if that looks good.

[hoxu]

Looks promising!

Is there a snapshot I could test before merging?

[yogthos]

Easiest if you just build locally by running lein install from the branch, that will install it in your local maven repo at ~/.m2/repository/migratus/migratus/1.3.3. And then projects using it will pull it from there.

[hoxu]

I tested this and glanced through the code changes. Some comments:

1. Is inject-properties? necessary? I think it would be enough to just check for presence of :custom-env-properties or :custom-properties, and do the substitutions if either exists. I actually forgot to set inject-properties? and spent some time debugging what's wrong.
2. It would be nice to get a warning (or even an exception) for un-substituted variables, because I imagine this is usually a user error. I realize current implementation does not allow for this, but perhaps open a separate issue for it to address later?
3. I recommend making whitespace/formatting changes in separate commits. This will make reviewing changes much easier for others (and you as well, when bisecting or digging the history later).

Otherwise looking good and does what I would expect, great work!

[yogthos]

Yeah that's a good point about inject-properties? being superfulous, I updated the config to look as follows:

:properties
 {:custom-env-properties [...]
  :custom-properties {...}}

And a good idea with the warning for any unmatched properties, agree that will help with the typos. I've updated the code that injects the properties to run a regex after substitution and generate warnings.

Generally, agree with the formatting sentiment but what's done is done. :)

Let me know if the latest looks good.

[hoxu]

Reviewed changes and tested the warning, too.

One last nitpick, about properties being repeated in the keys:

I would just rename the keys like so:

:properties
{:env [...]
 :map {...}}

[yogthos]

Sounds good, just pushed an update.

[hoxu]

Looks good to me!

[yogthos]

Excellent, just pushed out 1.3.4 to Clojars with the feature. 🎉

[hoxu]

Thank you!