From f13d6e68628235f0699e9f2b55cd5cb4889e217e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 23 Mar 2023 13:25:19 +0000 Subject: [PATCH] Revert "Merge pull request #33643 from libetl" This reverts commit 25e8f2d575f1f35d169df2cf48ac28a12d694af5, reversing changes made to e5bc9a2fcbe922330cb308c833c9b5cfe8046a38. Unfortunately, upon additional review we realised that these changes should not have been accepted. They're a partial implementation of support for programmatically configuring Logback, implemented in a way that only works during AOT processing and also potentially makes it harder for us to implement full support in the future. Closes gh-34361 --- .../logging/logback/LogbackLoggingSystem.java | 15 ++---- .../logback/SpringBootJoranConfigurator.java | 11 ----- .../logback/LogbackLoggingSystemTests.java | 49 +------------------ .../test/resources/logback-custom-rules.xml | 13 ----- 4 files changed, 6 insertions(+), 82 deletions(-) delete mode 100644 spring-boot-project/spring-boot/src/test/resources/logback-custom-rules.xml diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index acbab04259..ee31b9b07a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -20,8 +20,6 @@ import java.net.URL; import java.security.CodeSource; import java.security.ProtectionDomain; import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Set; import java.util.logging.ConsoleHandler; @@ -84,8 +82,6 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF private static final LogLevels LEVELS = new LogLevels<>(); - private Collection configurators = Collections.emptyList(); - static { LEVELS.map(LogLevel.TRACE, Level.TRACE); LEVELS.map(LogLevel.TRACE, Level.ALL); @@ -187,7 +183,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF if (isAlreadyInitialized(loggerContext)) { return; } - if (!initializeFromAotGeneratedArtifactsIfPossible(initializationContext, this.configurators, logFile)) { + if (!initializeFromAotGeneratedArtifactsIfPossible(initializationContext, logFile)) { super.initialize(initializationContext, configLocation, logFile); } loggerContext.getTurboFilterList().remove(FILTER); @@ -199,7 +195,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF } private boolean initializeFromAotGeneratedArtifactsIfPossible(LoggingInitializationContext initializationContext, - Collection configurators, LogFile logFile) { + LogFile logFile) { if (!AotDetector.useGeneratedArtifacts()) { return false; } @@ -208,8 +204,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF } LoggerContext loggerContext = getLoggerContext(); stopAndReset(loggerContext); - SpringBootJoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext, - configurators); + SpringBootJoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext); configurator.setContext(loggerContext); boolean configuredUsingAotGeneratedArtifacts = configurator.configureUsingAotGeneratedArtifacts(); if (configuredUsingAotGeneratedArtifacts) { @@ -272,7 +267,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF private void configureByResourceUrl(LoggingInitializationContext initializationContext, LoggerContext loggerContext, URL url) throws JoranException { if (url.toString().endsWith(".xml")) { - JoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext, this.configurators); + JoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext); configurator.setContext(loggerContext); configurator.doConfigure(url); } @@ -427,8 +422,6 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF BeanFactoryInitializationAotContribution contribution = (BeanFactoryInitializationAotContribution) context .getObject(key); context.removeObject(key); - this.configurators = beanFactory.getBeansOfType(JoranConfigurator.class).values(); - this.configurators.forEach((configurator) -> configurator.setContext(context)); return contribution; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/SpringBootJoranConfigurator.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/SpringBootJoranConfigurator.java index 6d5e3e2f93..c16a7d10b6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/SpringBootJoranConfigurator.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/SpringBootJoranConfigurator.java @@ -26,7 +26,6 @@ import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -79,16 +78,8 @@ class SpringBootJoranConfigurator extends JoranConfigurator { private final LoggingInitializationContext initializationContext; - private final Collection configurators; - SpringBootJoranConfigurator(LoggingInitializationContext initializationContext) { - this(initializationContext, Collections.emptyList()); - } - - SpringBootJoranConfigurator(LoggingInitializationContext initializationContext, - Collection configurators) { this.initializationContext = initializationContext; - this.configurators = configurators; } @Override @@ -114,7 +105,6 @@ class SpringBootJoranConfigurator extends JoranConfigurator { ruleStore.addRule(new ElementSelector("configuration/springProperty"), SpringPropertyAction::new); ruleStore.addRule(new ElementSelector("*/springProfile"), SpringProfileAction::new); ruleStore.addTransparentPathPart("springProfile"); - this.configurators.forEach((configurator) -> configurator.addElementSelectorAndActionAssociations(ruleStore)); } boolean configureUsingAotGeneratedArtifacts() { @@ -134,7 +124,6 @@ class SpringBootJoranConfigurator extends JoranConfigurator { getContext().putObject(BeanFactoryInitializationAotContribution.class.getName(), new LogbackConfigurationAotContribution(model, getModelInterpretationContext(), getContext())); } - this.configurators.forEach((configurator) -> configurator.processModel(model)); } private boolean isAotProcessingInProgress() { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index 78406ba46c..a0a27b077e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -33,15 +33,9 @@ import java.util.logging.LogManager; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.classic.spi.LoggerContextListener; import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.encoder.LayoutWrappingEncoder; -import ch.qos.logback.core.joran.action.Action; -import ch.qos.logback.core.joran.spi.ActionException; -import ch.qos.logback.core.joran.spi.ElementSelector; -import ch.qos.logback.core.joran.spi.RuleStore; -import ch.qos.logback.core.joran.spi.SaxEventInterpretationContext; import ch.qos.logback.core.rolling.RollingFileAppender; import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy; import ch.qos.logback.core.util.StatusPrinter; @@ -52,10 +46,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.ILoggerFactory; import org.slf4j.LoggerFactory; import org.slf4j.bridge.SLF4JBridgeHandler; -import org.xml.sax.Attributes; import org.springframework.beans.factory.aot.BeanFactoryInitializationAotContribution; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.boot.convert.ApplicationConversionService; import org.springframework.boot.logging.AbstractLoggingSystemTests; import org.springframework.boot.logging.LogFile; @@ -662,8 +654,7 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { @Test void whenContextHasNoAotContributionThenProcessAheadOfTimeReturnsNull() { - BeanFactoryInitializationAotContribution contribution = this.loggingSystem - .processAheadOfTime(new DefaultListableBeanFactory()); + BeanFactoryInitializationAotContribution contribution = this.loggingSystem.processAheadOfTime(null); assertThat(contribution).isNull(); } @@ -672,34 +663,11 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { LoggerContext context = ((LoggerContext) LoggerFactory.getILoggerFactory()); context.putObject(BeanFactoryInitializationAotContribution.class.getName(), mock(BeanFactoryInitializationAotContribution.class)); - BeanFactoryInitializationAotContribution contribution = this.loggingSystem - .processAheadOfTime(new DefaultListableBeanFactory()); + BeanFactoryInitializationAotContribution contribution = this.loggingSystem.processAheadOfTime(null); assertThat(context.getObject(BeanFactoryInitializationAotContribution.class.getName())).isNull(); assertThat(contribution).isNotNull(); } - @Test - void whenContextHasConfiguratorsThenInitializeExecutesThem(CapturedOutput output) { - LoggerContext context = ((LoggerContext) LoggerFactory.getILoggerFactory()); - context.putObject(BeanFactoryInitializationAotContribution.class.getName(), - mock(BeanFactoryInitializationAotContribution.class)); - DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); - beanFactory.registerSingleton("joranConfigurator1", new JoranConfigurator() { - - @Override - public void addElementSelectorAndActionAssociations(RuleStore ruleStore) { - ruleStore.addRule(new ElementSelector("*/rule1"), () -> new EmptyAction()); - ruleStore.addRule(new ElementSelector("*/rule2"), () -> new EmptyAction()); - } - - }); - this.loggingSystem.processAheadOfTime(beanFactory); - this.loggingSystem.beforeInitialize(); - initialize(this.initializationContext, "classpath:logback-custom-rules.xml", null); - assertThat(output).doesNotContain("Ignoring unknown property [rule1] in [ch.qos.logback.classic.LoggerContext]") - .doesNotContain("Ignoring unknown property [rule2] in [ch.qos.logback.classic.LoggerContext]"); - } - @Test // gh-33610 void springProfileIfNestedWithinSecondPhaseElementSanityChecker(CapturedOutput output) { this.loggingSystem.beforeInitialize(); @@ -742,17 +710,4 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { .orElse(null); } - private static class EmptyAction extends Action { - - @Override - public void begin(SaxEventInterpretationContext intercon, String name, Attributes attributes) - throws ActionException { - } - - @Override - public void end(SaxEventInterpretationContext intercon, String name) throws ActionException { - } - - } - } diff --git a/spring-boot-project/spring-boot/src/test/resources/logback-custom-rules.xml b/spring-boot-project/spring-boot/src/test/resources/logback-custom-rules.xml deleted file mode 100644 index beddbe5da7..0000000000 --- a/spring-boot-project/spring-boot/src/test/resources/logback-custom-rules.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - %property{LOG_FILE} [%t] ${PID:-????} %c{1}: %m%n BOOTBOOT - - - - - - - -