appender names of references not subject to substitution

ceki · ceki · commit 4f560a0331bc · 2026-02-05T19:32:51.000+01:00
Signed-off-by: ceki &lt;ceki@qos.ch&gt;
diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java
@@ -179,26 +179,24 @@ public void appenderRefSettingBySystemProperty() throws JoranException {
         configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "appenderRefByProperty.xml");
         final Logger logger = loggerContext.getLogger("ch.qos.logback.classic.joran");
         final ListAppender<ILoggingEvent> listAppender = (ListAppender<ILoggingEvent>) logger.getAppender("A");
-        assertEquals(0, listAppender.list.size());
+        // appender names or refs not subject to substitution
+        assertNull(listAppender);
+
         final String msg = "hello world";
         logger.info(msg);
 
-        assertEquals(1, listAppender.list.size());
         System.clearProperty(propertyName);
     }
+
     @Test
     public void appenderRefSettingBySystemPropertyDefault() throws JoranException {
 
         configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "appenderRefByPropertyDefault.xml");
-        StatusPrinter.print(loggerContext);
         final Logger logger = loggerContext.getLogger("ch.qos.logback.classic.joran");
         final ListAppender<ILoggingEvent> listAppender = (ListAppender<ILoggingEvent>) root.getAppender("A");
-        assertNotNull(listAppender);
-        assertEquals(0, listAppender.list.size());
-        final String msg = "hello world";
-        logger.info(msg);
+        // appender names not subjected to substitution
+        assertNull(listAppender);
 
-        assertEquals(1, listAppender.list.size());
     }
 
 
@@ -223,21 +221,20 @@ public void refToUndefinedAppender() throws JoranException {
     public void refViaDefaultSubstitution() throws JoranException {
 
         configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "refViaDefaultSubstitution.xml");
-        StatusPrinter.print(loggerContext);
         final Logger logger = loggerContext.getLogger("ch.qos.logback.classic.joran");
         final ListAppender<ILoggingEvent> listAppender = (ListAppender<ILoggingEvent>) root.getAppender("A");
         final NOPAppender<ILoggingEvent> nopAppender = (NOPAppender) root.getAppender("NOP");
 
         assertNotNull(listAppender);
-        assertNotNull(nopAppender);
+        assertNull(nopAppender);
 
         assertEquals(0, listAppender.list.size());
         final String msg = "hello world";
         logger.info(msg);
 
         assertEquals(1, listAppender.list.size());
 
-        checker.assertIsWarningOrErrorFree();
+        checker.assertContainsMatch("Appender named \\[\\$\\{NONEXISTENT:-NOP\\}\\] could not be found.");
     }
 
     @Test
diff --git a/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderModelHandler.java b/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderModelHandler.java
@@ -47,7 +47,8 @@ public void handle(ModelInterpretationContext mic, Model model) throws ModelHand
 
         AppenderModel appenderModel = (AppenderModel) model;
 
-        String appenderName = mic.subst(appenderModel.getName());
+        // appender names should not be subjected to substitution
+        String appenderName = appenderModel.getName();
 
         if (!mic.hasDependers(appenderName)) {
             addWarn("Appender named [" + appenderName + "] not referenced. Skipping further processing.");
diff --git a/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderRefDependencyAnalyser.java b/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderRefDependencyAnalyser.java
@@ -14,13 +14,21 @@
 package ch.qos.logback.core.model.processor;
 
 import ch.qos.logback.core.Context;
-import ch.qos.logback.core.model.AppenderModel;
 import ch.qos.logback.core.model.AppenderRefModel;
 import ch.qos.logback.core.model.Model;
 
 import java.util.List;
-import java.util.stream.Collectors;
 
+/**
+ * The AppenderRefDependencyAnalyser class is responsible for analyzing dependencies
+ * related to appender references within a logging model. This class extends
+ * ModelHandlerBase and operates during the dependency analysis phase of processing.
+ *
+ * The primary responsibilities of this class include:
+ * - Identifying instances of {@link AppenderRefModel} within a model hierarchy.
+ * - Substituting references to appender models using the context's interpretation logic.
+ * - Adding dependency definitions for the identified appender references to the interpretation context.
+ */
 @PhaseIndicator(phase = ProcessingPhase.DEPENDENCY_ANALYSIS)
 public class AppenderRefDependencyAnalyser extends ModelHandlerBase {
 
@@ -36,10 +44,11 @@ protected Class<Model> getSupportedModelClass() {
     @Override
     public void handle(ModelInterpretationContext mic, Model parentModel) throws ModelHandlerException {
         List<AppenderRefModel> appenderRefModelList = new java.util.ArrayList<>();
-        getAllAppenderRefModels(appenderRefModelList, parentModel);
+        collectAllAppenderRefModels(appenderRefModelList, parentModel);
 
         for (AppenderRefModel appenderRefModel : appenderRefModelList) {
-            String ref = mic.subst(appenderRefModel.getRef());
+            // TODO: prevent substitution of references
+            String ref = appenderRefModel.getRef();
             DependencyDefinition dd = new DependencyDefinition(parentModel, ref);
             mic.addDependencyDefinition(dd);
         }
@@ -53,13 +62,13 @@ public void handle(ModelInterpretationContext mic, Model parentModel) throws Mod
      * @param list the list to which AppenderRefModel instances are added
      * @param model the root Model object from which to start the extraction
      */
-    public void getAllAppenderRefModels(List<AppenderRefModel> list, Model model) {
+    public void collectAllAppenderRefModels(List<AppenderRefModel> list, Model model) {
         if(model == null)
             return;
         if(model instanceof AppenderRefModel) {
             list.add((AppenderRefModel) model);
         }
-        model.getSubModels().forEach(subModel -> getAllAppenderRefModels(list, subModel));
+        model.getSubModels().forEach(subModel -> collectAllAppenderRefModels(list, subModel));
     }
 
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderRefModelHandler.java b/logback-core/src/main/java/ch/qos/logback/core/model/processor/AppenderRefModelHandler.java
@@ -64,7 +64,8 @@ public void handle(ModelInterpretationContext interpContext, Model model) throws
     @SuppressWarnings({ "unchecked", "rawtypes" })
     void attachReferencedAppenders(ModelInterpretationContext mic, AppenderRefModel appenderRefModel,
             AppenderAttachable<?> appenderAttachable) {
-        String appenderName = mic.subst(appenderRefModel.getRef());
+        // appender ref should not be subject to substitution
+        String appenderName = appenderRefModel.getRef();
 
         if(!isAppenderDeclared(mic, appenderName)) {
             addWarn("Appender named [" + appenderName + "] could not be found. Skipping attachment to "+appenderAttachable+".");
