From ba51dc5c4a6822ee74af522546527d4326509343 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 22 Aug 2016 14:58:34 +0100 Subject: [PATCH] =?UTF-8?q?Make=20configuration=20of=20Liquibase=E2=80=99s?= =?UTF-8?q?=20logging=20more=20robust?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We make Liquibase aware of our custom Commons Logging-based logger by adding its package to the Liquibase ServiceLocator’s packages to scan. Previously, this was happening too late so Liquibase may have already initialized and cached a particular logger. This commit moves the registration of the extra package from the Liquibase auto-configuration to the application listener that customises Liquibase’s ServiceLocator. This ensures that the package is added before Liquibase is used. Unfortunately, configuring Liquibase’s ServiceLocator and its packages to scan causes it to try to perform some logging, resulting in it caching the wrong type of logger. We work around this problem by resetting Liquibase’s LogFactory once we’ve finished setting everything up. Closes gh-6713 --- .../liquibase/LiquibaseAutoConfiguration.java | 8 ++------ .../liquibase/LiquibaseAutoConfigurationTests.java | 14 ++++++++++++-- ...LiquibaseServiceLocatorApplicationListener.java | 10 +++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java index a55e3110de3..9ca197ecdd7 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,7 +21,6 @@ import javax.persistence.EntityManagerFactory; import javax.sql.DataSource; import liquibase.integration.spring.SpringLiquibase; -import liquibase.servicelocator.ServiceLocator; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -35,7 +34,6 @@ import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.autoconfigure.jdbc.DataSourceBuilder; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.boot.liquibase.CommonsLoggingLiquibaseLogger; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -52,6 +50,7 @@ import org.springframework.util.Assert; * @author Marcel Overdijk * @author Dave Syer * @author Phillip Webb + * @author Andy Wilkinson * @since 1.1.0 */ @Configuration @@ -87,9 +86,6 @@ public class LiquibaseAutoConfiguration { + " (please add changelog or check your Liquibase " + "configuration)"); } - ServiceLocator serviceLocator = ServiceLocator.getInstance(); - serviceLocator.addPackageToScan( - CommonsLoggingLiquibaseLogger.class.getPackage().getName()); } @Bean diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java index eb7a9655c14..38a95d7a9fd 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,33 +29,42 @@ import org.springframework.beans.factory.BeanCreationException; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration; import org.springframework.boot.liquibase.CommonsLoggingLiquibaseLogger; +import org.springframework.boot.liquibase.LiquibaseServiceLocatorApplicationListener; import org.springframework.boot.test.EnvironmentTestUtils; +import org.springframework.boot.test.OutputCapture; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.test.util.ReflectionTestUtils; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.contains; /** * Tests for {@link LiquibaseAutoConfiguration}. * * @author Marcel Overdijk + * @author Andy Wilkinson */ public class LiquibaseAutoConfigurationTests { @Rule public ExpectedException expected = ExpectedException.none(); + @Rule + public OutputCapture outputCapture = new OutputCapture(); + private AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); @Before public void init() { EnvironmentTestUtils.addEnvironment(this.context, "spring.datasource.name:liquibasetest"); + new LiquibaseServiceLocatorApplicationListener().onApplicationEvent(null); } @After @@ -166,7 +175,8 @@ public class LiquibaseAutoConfigurationTests { this.context.refresh(); SpringLiquibase liquibase = this.context.getBean(SpringLiquibase.class); Object log = ReflectionTestUtils.getField(liquibase, "log"); - assertThat(log, instanceOf(CommonsLoggingLiquibaseLogger.class)); + assertThat(log, instanceOf((CommonsLoggingLiquibaseLogger.class))); + assertThat(this.outputCapture.toString(), not(contains(": liquibase:"))); } @Test diff --git a/spring-boot/src/main/java/org/springframework/boot/liquibase/LiquibaseServiceLocatorApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/liquibase/LiquibaseServiceLocatorApplicationListener.java index a5d2502b317..6a32be7677d 100644 --- a/spring-boot/src/main/java/org/springframework/boot/liquibase/LiquibaseServiceLocatorApplicationListener.java +++ b/spring-boot/src/main/java/org/springframework/boot/liquibase/LiquibaseServiceLocatorApplicationListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,8 +51,12 @@ public class LiquibaseServiceLocatorApplicationListener private static class LiquibasePresent { public void replaceServiceLocator() { - ServiceLocator.setInstance(new CustomResolverServiceLocator( - new SpringPackageScanClassResolver(logger))); + CustomResolverServiceLocator customResolverServiceLocator = new CustomResolverServiceLocator( + new SpringPackageScanClassResolver(logger)); + customResolverServiceLocator.addPackageToScan( + CommonsLoggingLiquibaseLogger.class.getPackage().getName()); + ServiceLocator.setInstance(customResolverServiceLocator); + liquibase.logging.LogFactory.reset(); } }