Commit 1d2f6d25 authored by Phillip Webb's avatar Phillip Webb

Use consistent 'ROOT' logger name

Ensure all LoggingSystem implementation provide a consistent ROOT logger
name. Prior to this commit the `/loggers` endpoint could return '' for
root loggers which could then not be set using a POST.

Fixes gh-7372
parent 01c381f2
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
package org.springframework.boot.logging; package org.springframework.boot.logging;
import java.util.Comparator;
import java.util.HashMap; import java.util.HashMap;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Map; import java.util.Map;
...@@ -35,6 +36,9 @@ import org.springframework.util.SystemPropertyUtils; ...@@ -35,6 +36,9 @@ import org.springframework.util.SystemPropertyUtils;
*/ */
public abstract class AbstractLoggingSystem extends LoggingSystem { public abstract class AbstractLoggingSystem extends LoggingSystem {
protected static final Comparator<LoggerConfiguration> CONFIGURATION_COMPARATOR = new LoggerConfigurationComparator(
ROOT_LOGGER_NAME);
private final ClassLoader classLoader; private final ClassLoader classLoader;
public AbstractLoggingSystem(ClassLoader classLoader) { public AbstractLoggingSystem(ClassLoader classLoader) {
......
...@@ -25,9 +25,8 @@ import org.springframework.util.Assert; ...@@ -25,9 +25,8 @@ import org.springframework.util.Assert;
* Sorts the "root" logger as the first logger and then lexically by name after that. * Sorts the "root" logger as the first logger and then lexically by name after that.
* *
* @author Ben Hale * @author Ben Hale
* @since 1.5.0
*/ */
public class LoggerConfigurationComparator implements Comparator<LoggerConfiguration> { class LoggerConfigurationComparator implements Comparator<LoggerConfiguration> {
private final String rootLoggerName; private final String rootLoggerName;
...@@ -35,7 +34,7 @@ public class LoggerConfigurationComparator implements Comparator<LoggerConfigura ...@@ -35,7 +34,7 @@ public class LoggerConfigurationComparator implements Comparator<LoggerConfigura
* Create a new {@link LoggerConfigurationComparator} instance. * Create a new {@link LoggerConfigurationComparator} instance.
* @param rootLoggerName the name of the "root" logger * @param rootLoggerName the name of the "root" logger
*/ */
public LoggerConfigurationComparator(String rootLoggerName) { LoggerConfigurationComparator(String rootLoggerName) {
Assert.notNull(rootLoggerName, "RootLoggerName must not be null"); Assert.notNull(rootLoggerName, "RootLoggerName must not be null");
this.rootLoggerName = rootLoggerName; this.rootLoggerName = rootLoggerName;
} }
......
...@@ -350,7 +350,7 @@ public class LoggingApplicationListener implements GenericApplicationListener { ...@@ -350,7 +350,7 @@ public class LoggingApplicationListener implements GenericApplicationListener {
private void setLogLevel(LoggingSystem system, Environment environment, String name, private void setLogLevel(LoggingSystem system, Environment environment, String name,
String level) { String level) {
try { try {
if (name.equalsIgnoreCase("root")) { if (name.equalsIgnoreCase(LoggingSystem.ROOT_LOGGER_NAME)) {
name = null; name = null;
} }
level = environment.resolvePlaceholders(level); level = environment.resolvePlaceholders(level);
......
...@@ -47,6 +47,13 @@ public abstract class LoggingSystem { ...@@ -47,6 +47,13 @@ public abstract class LoggingSystem {
*/ */
public static final String NONE = "none"; public static final String NONE = "none";
/**
* The name used to for the root logger. LoggingSystem implementations should ensure
* that this is the name used to represent the root logger, regardless of the
* underlying implementation.
*/
public static final String ROOT_LOGGER_NAME = "ROOT";
private static final Map<String, String> SYSTEMS; private static final Map<String, String> SYSTEMS;
static { static {
...@@ -107,7 +114,8 @@ public abstract class LoggingSystem { ...@@ -107,7 +114,8 @@ public abstract class LoggingSystem {
/** /**
* Sets the logging level for a given logger. * Sets the logging level for a given logger.
* @param loggerName the name of the logger to set * @param loggerName the name of the logger to set ({@code null} can be used for the
* root logger).
* @param level the log level * @param level the log level
*/ */
public void setLogLevel(String loggerName, LogLevel level) { public void setLogLevel(String loggerName, LogLevel level) {
......
...@@ -31,7 +31,6 @@ import org.springframework.boot.logging.AbstractLoggingSystem; ...@@ -31,7 +31,6 @@ import org.springframework.boot.logging.AbstractLoggingSystem;
import org.springframework.boot.logging.LogFile; import org.springframework.boot.logging.LogFile;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggerConfigurationComparator;
import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.boot.logging.LoggingInitializationContext;
import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.logging.LoggingSystem;
import org.springframework.util.Assert; import org.springframework.util.Assert;
...@@ -49,9 +48,6 @@ import org.springframework.util.StringUtils; ...@@ -49,9 +48,6 @@ import org.springframework.util.StringUtils;
*/ */
public class JavaLoggingSystem extends AbstractLoggingSystem { public class JavaLoggingSystem extends AbstractLoggingSystem {
private static final LoggerConfigurationComparator COMPARATOR = new LoggerConfigurationComparator(
"");
private static final LogLevels<Level> LEVELS = new LogLevels<Level>(); private static final LogLevels<Level> LEVELS = new LogLevels<Level>();
static { static {
...@@ -122,8 +118,10 @@ public class JavaLoggingSystem extends AbstractLoggingSystem { ...@@ -122,8 +118,10 @@ public class JavaLoggingSystem extends AbstractLoggingSystem {
@Override @Override
public void setLogLevel(String loggerName, LogLevel level) { public void setLogLevel(String loggerName, LogLevel level) {
Assert.notNull(level, "Level must not be null"); Assert.notNull(level, "Level must not be null");
String name = (StringUtils.hasText(loggerName) ? loggerName : ""); if (loggerName == null || ROOT_LOGGER_NAME.equals(loggerName)) {
Logger logger = Logger.getLogger(name); loggerName = "";
}
Logger logger = Logger.getLogger(loggerName);
if (logger != null) { if (logger != null) {
logger.setLevel(LEVELS.convertSystemToNative(level)); logger.setLevel(LEVELS.convertSystemToNative(level));
} }
...@@ -136,7 +134,7 @@ public class JavaLoggingSystem extends AbstractLoggingSystem { ...@@ -136,7 +134,7 @@ public class JavaLoggingSystem extends AbstractLoggingSystem {
while (names.hasMoreElements()) { while (names.hasMoreElements()) {
result.add(getLoggerConfiguration(names.nextElement())); result.add(getLoggerConfiguration(names.nextElement()));
} }
Collections.sort(result, COMPARATOR); Collections.sort(result, CONFIGURATION_COMPARATOR);
return Collections.unmodifiableList(result); return Collections.unmodifiableList(result);
} }
...@@ -148,7 +146,9 @@ public class JavaLoggingSystem extends AbstractLoggingSystem { ...@@ -148,7 +146,9 @@ public class JavaLoggingSystem extends AbstractLoggingSystem {
} }
LogLevel level = LEVELS.convertNativeToSystem(logger.getLevel()); LogLevel level = LEVELS.convertNativeToSystem(logger.getLevel());
LogLevel effectiveLevel = LEVELS.convertNativeToSystem(getEffectiveLevel(logger)); LogLevel effectiveLevel = LEVELS.convertNativeToSystem(getEffectiveLevel(logger));
return new LoggerConfiguration(logger.getName(), level, effectiveLevel); String name = (StringUtils.hasLength(logger.getName()) ? logger.getName()
: ROOT_LOGGER_NAME);
return new LoggerConfiguration(name, level, effectiveLevel);
} }
private Level getEffectiveLevel(Logger root) { private Level getEffectiveLevel(Logger root) {
......
...@@ -41,7 +41,6 @@ import org.apache.logging.log4j.message.Message; ...@@ -41,7 +41,6 @@ import org.apache.logging.log4j.message.Message;
import org.springframework.boot.logging.LogFile; import org.springframework.boot.logging.LogFile;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggerConfigurationComparator;
import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.boot.logging.LoggingInitializationContext;
import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.logging.Slf4JLoggingSystem; import org.springframework.boot.logging.Slf4JLoggingSystem;
...@@ -61,9 +60,6 @@ import org.springframework.util.StringUtils; ...@@ -61,9 +60,6 @@ import org.springframework.util.StringUtils;
*/ */
public class Log4J2LoggingSystem extends Slf4JLoggingSystem { public class Log4J2LoggingSystem extends Slf4JLoggingSystem {
private static final LoggerConfigurationComparator COMPARATOR = new LoggerConfigurationComparator(
LogManager.ROOT_LOGGER_NAME);
private static final String FILE_PROTOCOL = "file"; private static final String FILE_PROTOCOL = "file";
private static final LogLevels<Level> LEVELS = new LogLevels<Level>(); private static final LogLevels<Level> LEVELS = new LogLevels<Level>();
...@@ -224,7 +220,7 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { ...@@ -224,7 +220,7 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem {
for (LoggerConfig loggerConfig : configuration.getLoggers().values()) { for (LoggerConfig loggerConfig : configuration.getLoggers().values()) {
result.add(convertLoggerConfiguration(loggerConfig)); result.add(convertLoggerConfiguration(loggerConfig));
} }
Collections.sort(result, COMPARATOR); Collections.sort(result, CONFIGURATION_COMPARATOR);
return result; return result;
} }
...@@ -238,7 +234,11 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { ...@@ -238,7 +234,11 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem {
return null; return null;
} }
LogLevel level = LEVELS.convertNativeToSystem(loggerConfig.getLevel()); LogLevel level = LEVELS.convertNativeToSystem(loggerConfig.getLevel());
return new LoggerConfiguration(loggerConfig.getName(), level, level); String name = loggerConfig.getName();
if (!StringUtils.hasLength(name) || LogManager.ROOT_LOGGER_NAME.equals(name)) {
name = ROOT_LOGGER_NAME;
}
return new LoggerConfiguration(name, level, level);
} }
@Override @Override
...@@ -254,7 +254,9 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { ...@@ -254,7 +254,9 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem {
} }
private LoggerConfig getLoggerConfig(String name) { private LoggerConfig getLoggerConfig(String name) {
name = (StringUtils.hasText(name) ? name : LogManager.ROOT_LOGGER_NAME); if (!StringUtils.hasLength(name) || ROOT_LOGGER_NAME.equals(name)) {
name = LogManager.ROOT_LOGGER_NAME;
}
return getLoggerContext().getConfiguration().getLoggers().get(name); return getLoggerContext().getConfiguration().getLoggers().get(name);
} }
......
...@@ -41,7 +41,6 @@ import org.slf4j.impl.StaticLoggerBinder; ...@@ -41,7 +41,6 @@ import org.slf4j.impl.StaticLoggerBinder;
import org.springframework.boot.logging.LogFile; import org.springframework.boot.logging.LogFile;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggerConfigurationComparator;
import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.boot.logging.LoggingInitializationContext;
import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.logging.Slf4JLoggingSystem; import org.springframework.boot.logging.Slf4JLoggingSystem;
...@@ -59,9 +58,6 @@ import org.springframework.util.StringUtils; ...@@ -59,9 +58,6 @@ import org.springframework.util.StringUtils;
*/ */
public class LogbackLoggingSystem extends Slf4JLoggingSystem { public class LogbackLoggingSystem extends Slf4JLoggingSystem {
private static final LoggerConfigurationComparator COMPARATOR = new LoggerConfigurationComparator(
Logger.ROOT_LOGGER_NAME);
private static final String CONFIGURATION_FILE_PROPERTY = "logback.configurationFile"; private static final String CONFIGURATION_FILE_PROPERTY = "logback.configurationFile";
private static final LogLevels<Level> LEVELS = new LogLevels<Level>(); private static final LogLevels<Level> LEVELS = new LogLevels<Level>();
...@@ -219,7 +215,7 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { ...@@ -219,7 +215,7 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem {
for (ch.qos.logback.classic.Logger logger : getLoggerContext().getLoggerList()) { for (ch.qos.logback.classic.Logger logger : getLoggerContext().getLoggerList()) {
result.add(getLoggerConfiguration(logger)); result.add(getLoggerConfiguration(logger));
} }
Collections.sort(result, COMPARATOR); Collections.sort(result, CONFIGURATION_COMPARATOR);
return result; return result;
} }
...@@ -236,7 +232,11 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { ...@@ -236,7 +232,11 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem {
LogLevel level = LEVELS.convertNativeToSystem(logger.getLevel()); LogLevel level = LEVELS.convertNativeToSystem(logger.getLevel());
LogLevel effectiveLevel = LEVELS LogLevel effectiveLevel = LEVELS
.convertNativeToSystem(logger.getEffectiveLevel()); .convertNativeToSystem(logger.getEffectiveLevel());
return new LoggerConfiguration(logger.getName(), level, effectiveLevel); String name = logger.getName();
if (!StringUtils.hasLength(name) || Logger.ROOT_LOGGER_NAME.equals(name)) {
name = ROOT_LOGGER_NAME;
}
return new LoggerConfiguration(name, level, effectiveLevel);
} }
@Override @Override
...@@ -259,7 +259,9 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { ...@@ -259,7 +259,9 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem {
private ch.qos.logback.classic.Logger getLogger(String name) { private ch.qos.logback.classic.Logger getLogger(String name) {
LoggerContext factory = getLoggerContext(); LoggerContext factory = getLoggerContext();
name = (StringUtils.isEmpty(name) ? Logger.ROOT_LOGGER_NAME : name); if (StringUtils.isEmpty(name) || ROOT_LOGGER_NAME.equals(name)) {
name = Logger.ROOT_LOGGER_NAME;
}
return factory.getLogger(name); return factory.getLogger(name);
} }
......
...@@ -33,6 +33,7 @@ import org.junit.Test; ...@@ -33,6 +33,7 @@ import org.junit.Test;
import org.springframework.boot.logging.AbstractLoggingSystemTests; import org.springframework.boot.logging.AbstractLoggingSystemTests;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.testutil.InternalOutputCapture; import org.springframework.boot.testutil.InternalOutputCapture;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
...@@ -175,7 +176,7 @@ public class JavaLoggingSystemTests extends AbstractLoggingSystemTests { ...@@ -175,7 +176,7 @@ public class JavaLoggingSystemTests extends AbstractLoggingSystemTests {
List<LoggerConfiguration> configurations = this.loggingSystem List<LoggerConfiguration> configurations = this.loggingSystem
.getLoggerConfigurations(); .getLoggerConfigurations();
assertThat(configurations).isNotEmpty(); assertThat(configurations).isNotEmpty();
assertThat(configurations.get(0).getName()).isEmpty(); assertThat(configurations.get(0).getName()).isEqualTo(LoggingSystem.ROOT_LOGGER_NAME);
} }
@Test @Test
......
...@@ -40,6 +40,7 @@ import org.junit.Test; ...@@ -40,6 +40,7 @@ import org.junit.Test;
import org.springframework.boot.logging.AbstractLoggingSystemTests; import org.springframework.boot.logging.AbstractLoggingSystemTests;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.testutil.InternalOutputCapture; import org.springframework.boot.testutil.InternalOutputCapture;
import org.springframework.boot.testutil.Matched; import org.springframework.boot.testutil.Matched;
import org.springframework.util.FileCopyUtils; import org.springframework.util.FileCopyUtils;
...@@ -148,7 +149,8 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { ...@@ -148,7 +149,8 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests {
List<LoggerConfiguration> configurations = this.loggingSystem List<LoggerConfiguration> configurations = this.loggingSystem
.getLoggerConfigurations(); .getLoggerConfigurations();
assertThat(configurations).isNotEmpty(); assertThat(configurations).isNotEmpty();
assertThat(configurations.get(0).getName()).isEmpty(); assertThat(configurations.get(0).getName())
.isEqualTo(LoggingSystem.ROOT_LOGGER_NAME);
} }
@Test @Test
......
...@@ -43,6 +43,7 @@ import org.springframework.boot.logging.LogFile; ...@@ -43,6 +43,7 @@ import org.springframework.boot.logging.LogFile;
import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LogLevel;
import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggerConfiguration;
import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.boot.logging.LoggingInitializationContext;
import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.testutil.InternalOutputCapture; import org.springframework.boot.testutil.InternalOutputCapture;
import org.springframework.boot.testutil.Matched; import org.springframework.boot.testutil.Matched;
import org.springframework.mock.env.MockEnvironment; import org.springframework.mock.env.MockEnvironment;
...@@ -190,7 +191,7 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { ...@@ -190,7 +191,7 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests {
.getLoggerConfigurations(); .getLoggerConfigurations();
assertThat(configurations).isNotEmpty(); assertThat(configurations).isNotEmpty();
assertThat(configurations.get(0).getName()) assertThat(configurations.get(0).getName())
.isEqualTo(org.slf4j.Logger.ROOT_LOGGER_NAME); .isEqualTo(LoggingSystem.ROOT_LOGGER_NAME);
} }
@Test @Test
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment