From 5302d91930d1676ba105dc7f7ab3a038601153c6 Mon Sep 17 00:00:00 2001 From: Leo Li <269739606@qq.com> Date: Thu, 28 Nov 2019 11:58:58 +0800 Subject: [PATCH 1/2] Fix Liquibase endpoint's output with multiple datasources Previously, the endpoint used the same change log history service for for each SpringLiquibase bean that it processed. This resulted in pollution of the reported changes as the history of each bean was not isolated. This commit updates the endpoint to use a new history service for each SpringLiquibase bean that is processed. See gh-19171 --- .../actuate/liquibase/LiquibaseEndpoint.java | 7 ++- .../liquibase/LiquibaseEndpointTests.java | 57 +++++++++++++++++++ .../changelog/db.changelog-master-backup.yaml | 20 +++++++ 3 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator/src/test/resources/db/changelog/db.changelog-master-backup.yaml diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java index 61d3a87220..612560e319 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java @@ -63,9 +63,10 @@ public class LiquibaseEndpoint { while (target != null) { Map liquibaseBeans = new HashMap<>(); DatabaseFactory factory = DatabaseFactory.getInstance(); - StandardChangeLogHistoryService service = new StandardChangeLogHistoryService(); - this.context.getBeansOfType(SpringLiquibase.class) - .forEach((name, liquibase) -> liquibaseBeans.put(name, createReport(liquibase, service, factory))); + this.context.getBeansOfType(SpringLiquibase.class).forEach((name, liquibase) -> { + StandardChangeLogHistoryService service = new StandardChangeLogHistoryService(); + liquibaseBeans.put(name, createReport(liquibase, service, factory)); + }); ApplicationContext parent = target.getParent(); contextBeans.put(target.getId(), new ContextLiquibaseBeans(liquibaseBeans, (parent != null) ? parent.getId() : null)); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java index f345c485a1..b10baa0851 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java @@ -22,6 +22,9 @@ import java.util.Map; import javax.sql.DataSource; +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; +import liquibase.integration.spring.SpringLiquibase; import org.junit.Test; import org.springframework.boot.actuate.liquibase.LiquibaseEndpoint.LiquibaseBean; @@ -41,6 +44,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Eddú Meléndez * @author Andy Wilkinson * @author Stephane Nicoll + * @author Leo Li */ public class LiquibaseEndpointTests { @@ -92,6 +96,20 @@ public class LiquibaseEndpointTests { }); } + @Test + public void multipleLiquibaseReportIsReturned() { + this.contextRunner.withUserConfiguration(Config.class, LiquibaseConfiguration.class).run((context) -> { + Map liquibaseBeans = context.getBean(LiquibaseEndpoint.class).liquibaseBeans() + .getContexts().get(context.getId()).getLiquibaseBeans(); + assertThat(liquibaseBeans.get("liquibase").getChangeSets()).hasSize(1); + assertThat(liquibaseBeans.get("liquibase").getChangeSets().get(0).getChangeLog()) + .isEqualTo("classpath:/db/changelog/db.changelog-master.yaml"); + assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets()).hasSize(1); + assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets().get(0).getChangeLog()) + .isEqualTo("classpath:/db/changelog/db.changelog-master-backup.yaml"); + }); + } + private boolean getAutoCommit(DataSource dataSource) throws SQLException { try (Connection connection = dataSource.getConnection()) { return connection.getAutoCommit(); @@ -108,4 +126,43 @@ public class LiquibaseEndpointTests { } + @Configuration + static class LiquibaseConfiguration { + + @Bean + DataSource dataSource() { + HikariConfig config = new HikariConfig(); + config.setJdbcUrl("jdbc:hsqldb:mem:test"); + config.setUsername("sa"); + return new HikariDataSource(config); + } + + @Bean + DataSource dataSourceBackup() { + HikariConfig config = new HikariConfig(); + config.setJdbcUrl("jdbc:hsqldb:mem:testBackup"); + config.setUsername("sa"); + return new HikariDataSource(config); + } + + @Bean + SpringLiquibase liquibase(DataSource dataSource) { + SpringLiquibase liquibase = new SpringLiquibase(); + liquibase.setChangeLog("classpath:/db/changelog/db.changelog-master.yaml"); + liquibase.setShouldRun(true); + liquibase.setDataSource(dataSource); + return liquibase; + } + + @Bean + SpringLiquibase liquibaseBackup(DataSource dataSourceBackup) { + SpringLiquibase liquibase = new SpringLiquibase(); + liquibase.setChangeLog("classpath:/db/changelog/db.changelog-master-backup.yaml"); + liquibase.setShouldRun(true); + liquibase.setDataSource(dataSourceBackup); + return liquibase; + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/resources/db/changelog/db.changelog-master-backup.yaml b/spring-boot-project/spring-boot-actuator/src/test/resources/db/changelog/db.changelog-master-backup.yaml new file mode 100644 index 0000000000..a3a19b43af --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/test/resources/db/changelog/db.changelog-master-backup.yaml @@ -0,0 +1,20 @@ +databaseChangeLog: + - changeSet: + id: 1 + author: leoli + changes: + - createTable: + tableName: customerbackup + columns: + - column: + name: id + type: int + autoIncrement: true + constraints: + primaryKey: true + nullable: false + - column: + name: name + type: varchar(50) + constraints: + nullable: false From e8eace2d5beff9e1131b9c5e022b8eb81d195f2c Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 29 Nov 2019 09:43:57 +0000 Subject: [PATCH 2/2] Polish "Fix Liquibase endpoint's output with multiple datasources" See gh-19171 --- .../actuate/liquibase/LiquibaseEndpoint.java | 11 ++-- .../liquibase/LiquibaseEndpointTests.java | 58 +++++++++---------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java index 612560e319..449542c5dc 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpoint.java @@ -25,7 +25,6 @@ import java.util.stream.Collectors; import javax.sql.DataSource; -import liquibase.changelog.ChangeLogHistoryService; import liquibase.changelog.ChangeSet.ExecType; import liquibase.changelog.RanChangeSet; import liquibase.changelog.StandardChangeLogHistoryService; @@ -63,10 +62,8 @@ public class LiquibaseEndpoint { while (target != null) { Map liquibaseBeans = new HashMap<>(); DatabaseFactory factory = DatabaseFactory.getInstance(); - this.context.getBeansOfType(SpringLiquibase.class).forEach((name, liquibase) -> { - StandardChangeLogHistoryService service = new StandardChangeLogHistoryService(); - liquibaseBeans.put(name, createReport(liquibase, service, factory)); - }); + this.context.getBeansOfType(SpringLiquibase.class) + .forEach((name, liquibase) -> liquibaseBeans.put(name, createReport(liquibase, factory))); ApplicationContext parent = target.getParent(); contextBeans.put(target.getId(), new ContextLiquibaseBeans(liquibaseBeans, (parent != null) ? parent.getId() : null)); @@ -75,8 +72,7 @@ public class LiquibaseEndpoint { return new ApplicationLiquibaseBeans(contextBeans); } - private LiquibaseBean createReport(SpringLiquibase liquibase, ChangeLogHistoryService service, - DatabaseFactory factory) { + private LiquibaseBean createReport(SpringLiquibase liquibase, DatabaseFactory factory) { try { DataSource dataSource = liquibase.getDataSource(); JdbcConnection connection = new JdbcConnection(dataSource.getConnection()); @@ -89,6 +85,7 @@ public class LiquibaseEndpoint { } database.setDatabaseChangeLogTableName(liquibase.getDatabaseChangeLogTable()); database.setDatabaseChangeLogLockTableName(liquibase.getDatabaseChangeLogLockTable()); + StandardChangeLogHistoryService service = new StandardChangeLogHistoryService(); service.setDatabase(database); return new LiquibaseBean( service.getRanChangeSets().stream().map(ChangeSet::new).collect(Collectors.toList())); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java index b10baa0851..8e4b16f086 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/liquibase/LiquibaseEndpointTests.java @@ -22,8 +22,6 @@ import java.util.Map; import javax.sql.DataSource; -import com.zaxxer.hikari.HikariConfig; -import com.zaxxer.hikari.HikariDataSource; import liquibase.integration.spring.SpringLiquibase; import org.junit.Test; @@ -31,10 +29,12 @@ import org.springframework.boot.actuate.liquibase.LiquibaseEndpoint.LiquibaseBea import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.autoconfigure.liquibase.LiquibaseAutoConfiguration; +import org.springframework.boot.jdbc.EmbeddedDatabaseConnection; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -97,17 +97,18 @@ public class LiquibaseEndpointTests { } @Test - public void multipleLiquibaseReportIsReturned() { - this.contextRunner.withUserConfiguration(Config.class, LiquibaseConfiguration.class).run((context) -> { - Map liquibaseBeans = context.getBean(LiquibaseEndpoint.class).liquibaseBeans() - .getContexts().get(context.getId()).getLiquibaseBeans(); - assertThat(liquibaseBeans.get("liquibase").getChangeSets()).hasSize(1); - assertThat(liquibaseBeans.get("liquibase").getChangeSets().get(0).getChangeLog()) - .isEqualTo("classpath:/db/changelog/db.changelog-master.yaml"); - assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets()).hasSize(1); - assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets().get(0).getChangeLog()) - .isEqualTo("classpath:/db/changelog/db.changelog-master-backup.yaml"); - }); + public void whenMultipleLiquibaseBeansArePresentChangeSetsAreCorrectlyReportedForEachBean() { + this.contextRunner.withUserConfiguration(Config.class, MultipleDataSourceLiquibaseConfiguration.class) + .run((context) -> { + Map liquibaseBeans = context.getBean(LiquibaseEndpoint.class) + .liquibaseBeans().getContexts().get(context.getId()).getLiquibaseBeans(); + assertThat(liquibaseBeans.get("liquibase").getChangeSets()).hasSize(1); + assertThat(liquibaseBeans.get("liquibase").getChangeSets().get(0).getChangeLog()) + .isEqualTo("classpath:/db/changelog/db.changelog-master.yaml"); + assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets()).hasSize(1); + assertThat(liquibaseBeans.get("liquibaseBackup").getChangeSets().get(0).getChangeLog()) + .isEqualTo("classpath:/db/changelog/db.changelog-master-backup.yaml"); + }); } private boolean getAutoCommit(DataSource dataSource) throws SQLException { @@ -127,39 +128,38 @@ public class LiquibaseEndpointTests { } @Configuration - static class LiquibaseConfiguration { + static class MultipleDataSourceLiquibaseConfiguration { @Bean DataSource dataSource() { - HikariConfig config = new HikariConfig(); - config.setJdbcUrl("jdbc:hsqldb:mem:test"); - config.setUsername("sa"); - return new HikariDataSource(config); + return createEmbeddedDatabase(); } @Bean DataSource dataSourceBackup() { - HikariConfig config = new HikariConfig(); - config.setJdbcUrl("jdbc:hsqldb:mem:testBackup"); - config.setUsername("sa"); - return new HikariDataSource(config); + return createEmbeddedDatabase(); } @Bean SpringLiquibase liquibase(DataSource dataSource) { - SpringLiquibase liquibase = new SpringLiquibase(); - liquibase.setChangeLog("classpath:/db/changelog/db.changelog-master.yaml"); - liquibase.setShouldRun(true); - liquibase.setDataSource(dataSource); - return liquibase; + return createSpringLiquibase("db.changelog-master.yaml", dataSource); } @Bean SpringLiquibase liquibaseBackup(DataSource dataSourceBackup) { + return createSpringLiquibase("db.changelog-master-backup.yaml", dataSourceBackup); + } + + private DataSource createEmbeddedDatabase() { + return new EmbeddedDatabaseBuilder().generateUniqueName(true) + .setType(EmbeddedDatabaseConnection.HSQL.getType()).build(); + } + + private SpringLiquibase createSpringLiquibase(String changeLog, DataSource dataSource) { SpringLiquibase liquibase = new SpringLiquibase(); - liquibase.setChangeLog("classpath:/db/changelog/db.changelog-master-backup.yaml"); + liquibase.setChangeLog("classpath:/db/changelog/" + changeLog); liquibase.setShouldRun(true); - liquibase.setDataSource(dataSourceBackup); + liquibase.setDataSource(dataSource); return liquibase; }