Skip to content

Add support for configuring Liquibase's tag #19316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add support for configuring Liquibase's tag #19316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...ain/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public SpringLiquibase liquibase(DataSourceProperties dataSourceProperties,
liquibase.setChangeLogParameters(this.properties.getParameters());
liquibase.setRollbackFile(this.properties.getRollbackFile());
liquibase.setTestRollbackOnUpdate(this.properties.isTestRollbackOnUpdate());
liquibase.setTag(this.properties.getTag());
return liquibase;
}

Expand Down
14 changes: 14 additions & 0 deletions ...e/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* Configuration properties to configure {@link SpringLiquibase}.
*
* @author Marcel Overdijk
* @author Eddú Meléndez
* @since 1.1.0
*/
@ConfigurationProperties(prefix = "spring.liquibase", ignoreUnknownFields = false)
Expand Down Expand Up @@ -114,6 +115,11 @@ public class LiquibaseProperties {
*/
private boolean testRollbackOnUpdate;

/**
* Name of the tag.
*/
private String tag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing this PR, I was trying to improve the documentation of that property and couldn't really find my way in the liquibase documentation. The original issue states:

This property will help in generating roll back scripts for particular tags only when user has specified rollbackFile property.

Do you know more about this property and how it's actually used? I've looked into the liquibase source code and I am not sure I understood the scope of that property. If that's used only for rollbacks, I think we should make that clearer in the description of the property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I was looking into the code I found SpringLiquibase uses tag property at performUpdate and generateRollbackFile methods. It means is executed during the update of changelogs or if additionally rollbackFile is also set will generate the file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. Same but I think "Name of the tag" isn't enough for users to figure out what to do. Perhaps this is an issue in the Liquibase documentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I poked around and I am confused. I don't think we should just have a "tag" property, I've commented on the original issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the late reply Stéphane! I have been thinking about it and TBH I think the workflow is pretty much deploying changes using the CLI and generating the rollback file from the application. As we have seen, the same property is used for rolling out and rollback. Said that, we should be very careful about the docs as you are worried about.

Delivering this feature, people will be able to:

  1. Use tag property, apply the tag value to the scripts

or

  1. Use tag property and rollbackFile, generate the file to perform a rollback

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveDonie thanks for the feedback and improving the docs.

We need a one sentence that would be a good summary for the property and we had a hard time finding something succinct.

Here is a proposal

Tag name to add to applied changes. Can be used with "rollbackFile" to generate a rollback script for all existing changes with that tag.

Thoughts?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is pretty good. Maybe even call out that there are two different uses:

Tag name to apply when applying database changes. Tag name to use as a target when rolling back already applied changes.

Copy link

@SteveDonie SteveDonie Feb 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When rolling back, you don't have to generate a rollbackFile. I'm not sure how it works in spring boot, but from the command line there are two commands - rollback, and rollbackSQL. One does the actual rollback, and the other generates the SQL to perform the rollback and shows it on the console (or sends it to a file).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for the pointers, @SteveDonie.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, thank you @SteveDonie. FTR, Spring Boot "merely" instruments SpringLiquibase which is part of Liquibase itself.


public String getChangeLog() {
return this.changeLog;
}
Expand Down Expand Up @@ -243,4 +249,12 @@ public void setTestRollbackOnUpdate(boolean testRollbackOnUpdate) {
this.testRollbackOnUpdate = testRollbackOnUpdate;
}

public String getTag() {
return this.tag;
}

public void setTag(String tag) {
this.tag = tag;
}

}
7 changes: 7 additions & 0 deletions ...ava/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,13 @@ void userConfigurationJdbcTemplateDependency() {
});
}

@Test
void overrideTag() {
this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class)
.withPropertyValues("spring.liquibase.tag:1.0.0")
.run(assertLiquibase((liquibase) -> assertThat(liquibase.getTag()).isEqualTo("1.0.0")));
}

private ContextConsumer<AssertableApplicationContext> assertLiquibase(Consumer<SpringLiquibase> consumer) {
return (context) -> {
assertThat(context).hasSingleBean(SpringLiquibase.class);
Expand Down