Rely soley on underlying logger for isEnabled in Liquibase logger

Previously, CommonsLoggingLiquibaseLogger referred to its
configured level and the underlying Commons Logging log when
determining if logging was enabled for a particular level. This did
not work as intended as setLogLevel was never called leaving the
configured level stuck at its default value of INFO. As a result of
this any logging at levels below INFO would not be output,
irrespective of the configuration of the underlying logging framework.

This commit updates CommonsLoggingLiquibaseLogger to rely purely on
the Commons Logging log when determining whether or not logging for
a particular level is enabled. This brings the implementation into
line with liquibase-slf4j [1] which provides similar functionality,
albeit using SLF4J rather than Commons Logging

Closes gh-2916

[1] https://github.com/mattbertolini/liquibase-slf4j/blob/master/src/main/java/liquibase/ext/logging/slf4j/Slf4jLogger.java
This commit is contained in:
Andy Wilkinson 2015-05-11 17:16:57 +01:00
parent 4977e48ec5
commit fc31115668
2 changed files with 5 additions and 47 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2014 the original author or authors.
* Copyright 2012-2015 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.
@ -16,11 +16,9 @@
package org.springframework.boot.liquibase;
import liquibase.configuration.LiquibaseConfiguration;
import liquibase.logging.LogLevel;
import liquibase.logging.Logger;
import liquibase.logging.core.AbstractLogger;
import liquibase.logging.core.DefaultLoggerConfiguration;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@ -30,6 +28,7 @@ import org.apache.commons.logging.LogFactory;
*
* @author Michael Cramer
* @author Phillip Webb
* @author Andy Wilkinson
* @since 1.2.0
*/
public class CommonsLoggingLiquibaseLogger extends AbstractLogger {
@ -119,7 +118,7 @@ public class CommonsLoggingLiquibaseLogger extends AbstractLogger {
}
private boolean isEnabled(LogLevel level) {
if (this.logger != null && getLogLevel().compareTo(level) <= 0) {
if (this.logger != null) {
switch (level) {
case DEBUG:
return this.logger.isDebugEnabled();
@ -134,14 +133,4 @@ public class CommonsLoggingLiquibaseLogger extends AbstractLogger {
return false;
}
@Override
public LogLevel getLogLevel() {
LogLevel logLevel = super.getLogLevel();
if (logLevel == null) {
return toLogLevel(LiquibaseConfiguration.getInstance()
.getConfiguration(DefaultLoggerConfiguration.class).getLogLevel());
}
return logLevel;
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2014 the original author or authors.
* Copyright 2012-2015 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.
@ -31,6 +31,7 @@ import static org.mockito.Mockito.verify;
* Tests for {@link CommonsLoggingLiquibaseLogger}.
*
* @author Phillip Webb
* @author Andy Wilkinson
*/
public class CommonsLoggingLiquibaseLoggerTests {
@ -70,14 +71,6 @@ public class CommonsLoggingLiquibaseLoggerTests {
verify(this.delegate, never()).debug("debug");
}
@Test
public void debugBelowLevel() {
this.logger.setLogLevel(LogLevel.INFO);
given(this.delegate.isDebugEnabled()).willReturn(true);
this.logger.debug("debug", this.ex);
verify(this.delegate, never()).debug("debug", this.ex);
}
@Test
public void info() {
this.logger.setLogLevel(LogLevel.INFO);
@ -102,14 +95,6 @@ public class CommonsLoggingLiquibaseLoggerTests {
verify(this.delegate, never()).info("info");
}
@Test
public void infoBelowLevel() {
this.logger.setLogLevel(LogLevel.WARNING);
given(this.delegate.isInfoEnabled()).willReturn(true);
this.logger.info("info", this.ex);
verify(this.delegate, never()).info("info", this.ex);
}
@Test
public void warning() {
this.logger.setLogLevel(LogLevel.WARNING);
@ -134,14 +119,6 @@ public class CommonsLoggingLiquibaseLoggerTests {
verify(this.delegate, never()).warn("warning");
}
@Test
public void warningBelowLevel() {
this.logger.setLogLevel(LogLevel.SEVERE);
given(this.delegate.isWarnEnabled()).willReturn(true);
this.logger.warning("warning", this.ex);
verify(this.delegate, never()).warn("warning", this.ex);
}
@Test
public void severe() {
this.logger.setLogLevel(LogLevel.SEVERE);
@ -166,14 +143,6 @@ public class CommonsLoggingLiquibaseLoggerTests {
verify(this.delegate, never()).error("severe");
}
@Test
public void severeBelowLevel() {
this.logger.setLogLevel(LogLevel.OFF);
given(this.delegate.isErrorEnabled()).willReturn(true);
this.logger.severe("severe", this.ex);
verify(this.delegate, never()).error("severe", this.ex);
}
private class MockCommonsLoggingLiquibaseLogger extends CommonsLoggingLiquibaseLogger {
@Override