fix sequenceNumberGeneration

Signed-off-by: Ceki Gulcu <ceki@qos.ch>
qos-ch · Aug 22, 2022 · 255af5a · 255af5a
1 parent d2c4044
commit 255af5a
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 40 deletions.
diff --git a/logback-classic/src/test/input/joran/sequenceNumberGenerator-missingClass.xml b/logback-classic/src/test/input/joran/sequenceNumberGenerator-missingClass.xml
@@ -0,0 +1,10 @@
+<configuration >
+
+  <sequenceNumberGenerator/>
+
+  <appender name="LIST" class="ch.qos.logback.core.read.ListAppender"/>
+
+  <root level="debug">
+    <appender-ref ref="LIST" />
+  </root>
+</configuration>
diff --git a/logback-classic/src/test/input/joran/sequenceNumberGenerator.xml b/logback-classic/src/test/input/joran/sequenceNumberGenerator.xml
@@ -0,0 +1,10 @@
+<configuration >
+
+  <sequenceNumberGenerator class="ch.qos.logback.core.spi.BasicSequenceNumberGenerator"/>
+
+  <appender name="LIST" class="ch.qos.logback.core.read.ListAppender"/>
+
+  <root level="debug">
+    <appender-ref ref="LIST" />
+  </root>
+</configuration>
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
@@ -630,6 +630,33 @@ public void missingPropertyErrorHandling() throws JoranException {
     }
 
     @Test
+    public void sequenceNumberGenerator() throws JoranException {
+        configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "sequenceNumberGenerator.xml");
+        final ListAppender<ILoggingEvent> listAppender= (ListAppender<ILoggingEvent>) root.getAppender("LIST");
+        assertNotNull(listAppender);
+
+        logger.atDebug().setMessage("hello").log();
+        logger.atDebug().setMessage("world").log();
+
+        ILoggingEvent le0 = listAppender.list.get(0);
+        ILoggingEvent le1 = listAppender.list.get(1);
+
+        long se0 = le0.getSequenceNumber();
+        long se1 = le1.getSequenceNumber();
+        assertEquals(1, se1 - se0);
+        StatusPrinter.print(loggerContext);
+    }
+
+    @Test
+    public void sequenceNumberGenerator_missingClass() throws JoranException {
+        configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "sequenceNumberGenerator-missingClass.xml");
+        StatusPrinter.print(loggerContext);
+        final ListAppender<ILoggingEvent> listAppender= (ListAppender<ILoggingEvent>) root.getAppender("LIST");
+        assertNotNull(listAppender);
+        checker.assertContainsMatch(Status.ERROR, "Missing attribute \\[class\\] in element \\[sequenceNumberGenerator\\]");
+    }
+
+        @Test
     public void kvp() throws JoranException {
         configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "pattern/kvp.xml");
 

diff --git a/logback-core/src/main/java/ch/qos/logback/core/joran/JoranConfiguratorBase.java b/logback-core/src/main/java/ch/qos/logback/core/joran/JoranConfiguratorBase.java
@@ -13,52 +13,19 @@
  */
 package ch.qos.logback.core.joran;
 
-import ch.qos.logback.core.joran.action.AppenderAction;
-import ch.qos.logback.core.joran.action.AppenderRefAction;
-import ch.qos.logback.core.joran.action.ContextPropertyAction;
-import ch.qos.logback.core.joran.action.ConversionRuleAction;
-import ch.qos.logback.core.joran.action.DefinePropertyAction;
-import ch.qos.logback.core.joran.action.EventEvaluatorAction;
-import ch.qos.logback.core.joran.action.ImplicitModelAction;
-import ch.qos.logback.core.joran.action.ImportAction;
-import ch.qos.logback.core.joran.action.NewRuleAction;
-import ch.qos.logback.core.joran.action.ParamAction;
-import ch.qos.logback.core.joran.action.PropertyAction;
-import ch.qos.logback.core.joran.action.ShutdownHookAction;
-import ch.qos.logback.core.joran.action.SiftAction;
-import ch.qos.logback.core.joran.action.StatusListenerAction;
-import ch.qos.logback.core.joran.action.TimestampAction;
+import ch.qos.logback.core.joran.action.*;
 import ch.qos.logback.core.joran.conditional.ElseAction;
 import ch.qos.logback.core.joran.conditional.IfAction;
 import ch.qos.logback.core.joran.conditional.ThenAction;
 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.joran.spi.SaxEventInterpreter;
-import ch.qos.logback.core.model.DefineModel;
-import ch.qos.logback.core.model.EventEvaluatorModel;
-import ch.qos.logback.core.model.ImplicitModel;
-import ch.qos.logback.core.model.ImportModel;
-import ch.qos.logback.core.model.IncludeModel;
-import ch.qos.logback.core.model.ParamModel;
-import ch.qos.logback.core.model.PropertyModel;
-import ch.qos.logback.core.model.ShutdownHookModel;
-import ch.qos.logback.core.model.SiftModel;
-import ch.qos.logback.core.model.StatusListenerModel;
-import ch.qos.logback.core.model.TimestampModel;
+import ch.qos.logback.core.model.*;
 import ch.qos.logback.core.model.conditional.ElseModel;
 import ch.qos.logback.core.model.conditional.IfModel;
 import ch.qos.logback.core.model.conditional.ThenModel;
-import ch.qos.logback.core.model.processor.DefaultProcessor;
-import ch.qos.logback.core.model.processor.DefineModelHandler;
-import ch.qos.logback.core.model.processor.EventEvaluatorModelHandler;
-import ch.qos.logback.core.model.processor.ImplicitModelHandler;
-import ch.qos.logback.core.model.processor.ImportModelHandler;
-import ch.qos.logback.core.model.processor.NOPModelHandler;
-import ch.qos.logback.core.model.processor.PropertyModelHandler;
-import ch.qos.logback.core.model.processor.ShutdownHookModelHandler;
-import ch.qos.logback.core.model.processor.StatusListenerModelHandler;
-import ch.qos.logback.core.model.processor.TimestampModelHandler;
+import ch.qos.logback.core.model.processor.*;
 import ch.qos.logback.core.model.processor.conditional.ElseModelHandler;
 import ch.qos.logback.core.model.processor.conditional.IfModelHandler;
 import ch.qos.logback.core.model.processor.conditional.ThenModelHandler;
@@ -93,6 +60,8 @@ protected void addElementSelectorAndActionAssociations(RuleStore rs) {
 
         rs.addRule(new ElementSelector("configuration/timestamp"),  TimestampAction::new);
         rs.addRule(new ElementSelector("configuration/shutdownHook"),  ShutdownHookAction::new);
+        rs.addRule(new ElementSelector("configuration/sequenceNumberGenerator"),  SequenceNumberGeneratorAction::new);
+
         rs.addRule(new ElementSelector("configuration/define"),  DefinePropertyAction::new);
         rs.addRule(new ElementSelector("configuration/evaluator"),  EventEvaluatorAction::new);
 
@@ -144,6 +113,8 @@ protected void addModelHandlerAssociations(DefaultProcessor defaultProcessor) {
         defaultProcessor.addHandler(ImportModel.class, ImportModelHandler::makeInstance);
 
         defaultProcessor.addHandler(ShutdownHookModel.class, ShutdownHookModelHandler::makeInstance);
+        defaultProcessor.addHandler(SequenceNumberGeneratorModel.class, SequenceNumberGeneratorModelHandler::makeInstance);
+
         defaultProcessor.addHandler(EventEvaluatorModel.class, EventEvaluatorModelHandler::makeInstance);
         defaultProcessor.addHandler(DefineModel.class, DefineModelHandler::makeInstance);
         defaultProcessor.addHandler(IncludeModel.class, NOPModelHandler::makeInstance);

diff --git a/...rc/main/java/ch/qos/logback/core/model/processor/SequenceNumberGeneratorModelHandler.java b/...rc/main/java/ch/qos/logback/core/model/processor/SequenceNumberGeneratorModelHandler.java
@@ -30,7 +30,7 @@ public SequenceNumberGeneratorModelHandler(Context context) {
     }
 
     static public ModelHandlerBase makeInstance(Context context, ModelInterpretationContext ic) {
-        return new ShutdownHookModelHandler(context);
+        return new SequenceNumberGeneratorModelHandler(context);
     }
 
     @Override
@@ -44,8 +44,9 @@ public void handle(ModelInterpretationContext mic, Model model) throws ModelHand
         SequenceNumberGeneratorModel sequenceNumberGeneratorModel = (SequenceNumberGeneratorModel) model;
         String className = sequenceNumberGeneratorModel.getClassName();
         if (OptionHelper.isNullOrEmpty(className)) {
-            className = BasicSequenceNumberGenerator.class.getName();
-            addInfo("Assuming className [" + className + "]");
+            addWarn("Missing className. This should have been caught earlier.");
+            inError = true;
+            return;
         } else {
             className = mic.getImport(className);
         }
@@ -76,7 +77,7 @@ public void postHandle(ModelInterpretationContext mic, Model model) throws Model
         } else {
             mic.popObject();
 
-            addInfo("Registering sequenceNumberGenerator with context.");
+            addInfo("Registering "+o+" with context.");
             context.setSequenceNumberGenerator(sequenceNumberGenerator);
         }
     }