This is an automated email from the ASF dual-hosted git repository. jbonofre pushed a commit to branch pull/1948/head in repository https://gitbox.apache.org/repos/asf/karaf.git
commit 6d9a4611a45abe0f6e48f1975e048803cb1ce0bd Author: Antti Nevala <[email protected]> AuthorDate: Fri May 9 09:40:29 2025 +0300 Fix LogServiceLogback to find root logger correctly and add support for basic variable substitution --- .../core/internal/LogServiceLogbackXmlImpl.java | 100 ++++++++++++-- .../core/internal/LogServiceLog4j2XmlImplTest.java | 102 +++++++-------- .../core/internal/LogServiceLogbackXmlTest.java | 144 ++++++++++++++++++--- log/src/test/resources/logback.xml | 28 ++++ 4 files changed, 291 insertions(+), 83 deletions(-) diff --git a/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java b/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java index ad4924aad2..89f41aec9a 100644 --- a/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java +++ b/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java @@ -39,17 +39,19 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Map; -import java.util.TreeMap; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class LogServiceLogbackXmlImpl implements LogServiceInternal { private static final String ELEMENT_ROOT = "root"; private static final String ELEMENT_LOGGER = "logger"; + private static final String ELEMENT_PROPERTY = "property"; + private static final String ELEMENT_VARIABLE = "variable"; private static final String ATTRIBUTE_NAME = "name"; private static final String ATTRIBUTE_LEVEL = "level"; + private static final String ATTRIBUTE_VALUE = "value"; private static final String ELEMENT_CONFIGURATION = "configuration"; private final Path path; @@ -61,13 +63,15 @@ public class LogServiceLogbackXmlImpl implements LogServiceInternal { public Map<String, String> getLevel(String logger) { try { Document doc = loadConfig(path); + Map<String, String> properties = getProperties(doc); Map<String, Element> loggers = getLoggers(doc); Map<String, String> levels = new TreeMap<>(); for (Map.Entry<String, Element> e : loggers.entrySet()) { + String level = e.getValue().getAttribute(ATTRIBUTE_LEVEL); - if (level != null && !level.isEmpty()) { - levels.put(e.getKey(), level); + if (!level.isEmpty()) { + levels.put(e.getKey(), resolveValue(level, properties, new Properties(System.getProperties()), new HashMap<>(System.getenv()))); } } @@ -93,6 +97,40 @@ public class LogServiceLogbackXmlImpl implements LogServiceInternal { } } + static String resolveValue(String value, Map<String, String> properties, Properties systemProperties, Map<String, String> envVariables) { + // Pattern to match ${variable:-default} + // At this moment only basic substitution is supported + // i.e D${my.param:-EBUG} is not supported + Pattern pattern = Pattern.compile("\\$\\{(.+?)(?::-(.+?))?}"); + Matcher matcher = pattern.matcher(value); + + if (matcher.matches()) { + String variable = matcher.group(1); + String defaultValue = matcher.group(2); + // Remove found property to prevent cyclic loops + // Check properties + String resolved = properties.remove(variable); + if (resolved == null) { + // Check system properties + resolved = systemProperties.getProperty(variable); + systemProperties.remove(variable); + } + if (resolved == null) { + // Check environment variables + resolved = envVariables.remove(variable); + } + + if (resolved != null) { + // Check resolved variable again to susbstitute + return resolveValue(resolved, properties, systemProperties, envVariables); + } else { + return defaultValue; + } + } else { + return value; + } + } + public void setLevel(String logger, String level) { try { Document doc = loadConfig(path); @@ -146,13 +184,13 @@ public class LogServiceLogbackXmlImpl implements LogServiceInternal { static void insertIndented(Element parent, Element element) { NodeList taggedElements = parent.getElementsByTagName("*"); //only use direct descendants of parent element to insert next to - ArrayList <Node> childElements = new ArrayList<Node>(); + ArrayList <Node> childElements = new ArrayList<>(); for (int i = 0;i < taggedElements.getLength(); i++ ){ if(taggedElements.item(i).getParentNode().equals(parent)){ childElements.add(taggedElements.item(i)); } } - Node insertAfter = childElements.size() > 0 ? childElements.get(childElements.size() - 1) : null; + Node insertAfter = !childElements.isEmpty() ? childElements.get(childElements.size() - 1) : null; if (insertAfter != null) { if (insertAfter.getPreviousSibling() != null && insertAfter.getPreviousSibling().getNodeType() == Node.TEXT_NODE) { String indent = insertAfter.getPreviousSibling().getTextContent(); @@ -185,7 +223,7 @@ public class LogServiceLogbackXmlImpl implements LogServiceInternal { indent += "\t"; } } - if (parent.getFirstChild() != null && parent.getPreviousSibling().getNodeType() == Node.TEXT_NODE) { + if (parent.getFirstChild() != null) { parent.removeChild(parent.getFirstChild()); } } else { @@ -250,17 +288,53 @@ public class LogServiceLogbackXmlImpl implements LogServiceInternal { Node n = loggersList.item(i); if (n instanceof Element) { Element e = (Element) n; - if (ELEMENT_ROOT.equals(e.getLocalName())) { - loggers.put(ROOT_LOGGER, e); - } else if (ELEMENT_LOGGER.equals(e.getLocalName())) { + if (ELEMENT_LOGGER.equals(e.getLocalName())) { String name = e.getAttribute(ATTRIBUTE_NAME); - if (name != null) { + if (!name.isEmpty()) { loggers.put(name, e); } } } } + // Handle root separately + Node n = docE.getElementsByTagName(ELEMENT_ROOT).item(0); + if (n instanceof Element) { + Element e = (Element) n; + if (ELEMENT_ROOT.equals(e.getLocalName())) { + loggers.put(ROOT_LOGGER, e); + } + } return loggers; } + private Map<String, String> getProperties(Document doc) { + Map<String, String> properties = new TreeMap<>(); + Element docE = doc.getDocumentElement(); + if (!ELEMENT_CONFIGURATION.equals(docE.getLocalName())) { + throw new IllegalArgumentException("Xml root document should be " + ELEMENT_CONFIGURATION); + } + NodeList propertyList = docE.getElementsByTagName(ELEMENT_PROPERTY); + extractProperties(propertyList, ELEMENT_PROPERTY, properties); + NodeList variableList = docE.getElementsByTagName(ELEMENT_VARIABLE); + extractProperties(variableList, ELEMENT_VARIABLE, properties); + return properties; + } + + private static void extractProperties(NodeList propertyList, String elementName, Map<String, String> properties) { + for (int i = 0; i < propertyList.getLength(); i++) { + Node n = propertyList.item(i); + if (n instanceof Element) { + Element e = (Element) n; + if (elementName.equals(e.getLocalName())) { + String name = e.getAttribute(ATTRIBUTE_NAME); + String value = e.getAttribute(ATTRIBUTE_VALUE); + if (!name.isEmpty()) { + properties.put(name, value); + } + } + } + } + } + + } diff --git a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java index ce6a770e79..1b0815cdf7 100644 --- a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java +++ b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java @@ -34,99 +34,99 @@ public class LogServiceLog4j2XmlImplTest { @Test public void testInsertIndentedTabs() throws Exception { - String xml = "<Configuration>\n" + - "\t<Loggers>\n" + - "\t</Loggers>\n" + + String xml = "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>"; String out = insertIndented(xml, false); assertEquals( - "<Configuration>\n" + - "\t<Loggers>\n" + - "\t\t<Logger/>\n" + - "\t</Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t\t<Logger/>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>", out); out = insertIndented(xml, true); assertEquals( - "<Configuration>\n" + - "\t<Loggers>\n" + - "\t\t<Logger/>\n" + - "\t</Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t\t<Logger/>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>", out); } @Test public void testInsertIndentedSpaces() throws Exception { - String xml = "<Configuration>\n" + - " <Loggers>\n" + - " </Loggers>\n" + + String xml = "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>"; String out = insertIndented(xml, false); assertEquals( - "<Configuration>\n" + - " <Loggers>\n" + - " <Logger/>\n" + - " </Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " <Logger/>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>", out); out = insertIndented(xml, true); assertEquals( - "<Configuration>\n" + - " <Loggers>\n" + - " <Logger/>\n" + - " </Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " <Logger/>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>", out); } @Test public void testInsertIndentedTabsWithRoot() throws Exception { - String xml = "<Configuration>\n" + - "\t<Loggers>\n" + - "\t\t<Root/>\n" + - "\t</Loggers>\n" + + String xml = "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t\t<Root/>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>"; String out = insertIndented(xml, false); assertEquals( - "<Configuration>\n" + - "\t<Loggers>\n" + - "\t\t<Root/>\n" + - "\t\t<Logger/>\n" + - "\t</Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t\t<Root/>" + System.lineSeparator() + + "\t\t<Logger/>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>", out); out = insertIndented(xml, true); assertEquals( - "<Configuration>\n" + - "\t<Loggers>\n" + - "\t\t<Logger/>\n" + - "\t\t<Root/>\n" + - "\t</Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + "\t<Loggers>" + System.lineSeparator() + + "\t\t<Logger/>" + System.lineSeparator() + + "\t\t<Root/>" + System.lineSeparator() + + "\t</Loggers>" + System.lineSeparator() + "</Configuration>", out); } @Test public void testInsertIndentedSpacesWithRoot() throws Exception { - String xml = "<Configuration>\n" + - " <Loggers>\n" + - " <Root/>\n" + - " </Loggers>\n" + + String xml = "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " <Root/>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>"; - + String out = insertIndented(xml, false); assertEquals( - "<Configuration>\n" + - " <Loggers>\n" + - " <Root/>\n" + - " <Logger/>\n" + - " </Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " <Root/>" + System.lineSeparator() + + " <Logger/>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>", out); out = insertIndented(xml, true); assertEquals( - "<Configuration>\n" + - " <Loggers>\n" + - " <Logger/>\n" + - " <Root/>\n" + - " </Loggers>\n" + + "<Configuration>" + System.lineSeparator() + + " <Loggers>" + System.lineSeparator() + + " <Logger/>" + System.lineSeparator() + + " <Root/>" + System.lineSeparator() + + " </Loggers>" + System.lineSeparator() + "</Configuration>", out); } diff --git a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java index 70b62571a6..e3d8a537a1 100644 --- a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java +++ b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java @@ -16,6 +16,7 @@ */ package org.apache.karaf.log.core.internal; +import org.junit.BeforeClass; import org.junit.Test; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -27,63 +28,85 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import java.io.ByteArrayInputStream; import java.io.StringWriter; +import java.net.URISyntaxException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class LogServiceLogbackXmlTest { + public static final String LOG_LEVEL_TOKEN = "log.level"; + + private static String file; + @BeforeClass + public static void initClass() { + Path p; + try { + p = Paths.get(LogServiceLogbackXmlImpl.class.getResource("/logback.xml").toURI()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + file = p.toString(); + } + @Test public void testInsertIndentedTabs() throws Exception { - String xml = "<configuration>\n" + + String xml = "<configuration>" + System.lineSeparator() + "</configuration>"; String out = insertIndented(xml); assertEquals( - "<configuration>\n" + - "\t<logger/>\n" + + "<configuration>" + System.lineSeparator() + + "\t<logger/>" + System.lineSeparator() + "</configuration>", out); } @Test public void testInsertIndentedSpaces() throws Exception { //this one tests with one logger already added, because with no loggers there is no indentation to decide by and the function will choose tab - String xml = "<configuration>\n" + - " <logger/>\n" + + String xml = "<configuration>" + System.lineSeparator() + + " <logger/>" + System.lineSeparator() + "</configuration>"; String out = insertIndented(xml); assertEquals( - "<configuration>\n" + - " <logger/>\n" + - " <logger/>\n" + + "<configuration>" + System.lineSeparator() + + " <logger/>" + System.lineSeparator() + + " <logger/>" + System.lineSeparator() + "</configuration>", out); } @Test public void testInsertIndentedTabsWithRoot() throws Exception { - String xml = "<configuration>\n" + - "\t<root/>\n" + + String xml = "<configuration>" + System.lineSeparator() + + "\t<root/>" + System.lineSeparator() + "</configuration>"; String out = insertIndented(xml); assertEquals( - "<configuration>\n" + - "\t<root/>\n" + - "\t<logger/>\n" + + "<configuration>" + System.lineSeparator() + + "\t<root/>" + System.lineSeparator() + + "\t<logger/>" + System.lineSeparator() + "</configuration>", out); } @Test public void testInsertIndentedSpacesWithRoot() throws Exception { - String xml = "<configuration>\n" + - " <root/>\n" + + String xml = "<configuration>" + System.lineSeparator() + + " <root/>" + System.lineSeparator() + "</configuration>"; String out = insertIndented(xml); assertEquals( - "<configuration>\n" + - " <root/>\n" + - " <logger/>\n" + + "<configuration>" + System.lineSeparator() + + " <root/>" + System.lineSeparator() + + " <logger/>" + System.lineSeparator() + "</configuration>", out); } @@ -91,7 +114,7 @@ public class LogServiceLogbackXmlTest { Document doc = LogServiceLog4j2XmlImpl.loadConfig(null, new ByteArrayInputStream(xml.getBytes())); Element element = doc.createElement("logger"); LogServiceLogbackXmlImpl.insertIndented( - (Element) doc.getDocumentElement(), + doc.getDocumentElement(), element); try (StringWriter os = new StringWriter()) { TransformerFactory tFactory = TransformerFactory.newInstance(); @@ -101,4 +124,87 @@ public class LogServiceLogbackXmlTest { return os.toString(); } } + + @Test + public void testBasicValue() { + Properties systemProps = new Properties(); + String resolved = LogServiceLogbackXmlImpl.resolveValue("DEBUG", Collections.emptyMap(), systemProps, Collections.emptyMap()); + assertEquals("DEBUG", resolved); + } + + @Test + public void testPropertySubstitution() { + Map<String, String> properties = new HashMap<>(); + properties.put(LOG_LEVEL_TOKEN, "WARN"); + String resolved = LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}", properties, new Properties(), Collections.emptyMap()); + assertEquals("WARN", resolved); + } + + @Test + public void testSystemPropertiesSubstitution() { + Properties systemProps = new Properties(); + systemProps.put(LOG_LEVEL_TOKEN, "WARN"); + String resolved = LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}", Collections.emptyMap(), systemProps, Collections.emptyMap()); + assertEquals("WARN", resolved); + } + + @Test + public void testEnvVariableSubstitution() { + Map<String, String> env = new HashMap<>(); + env.put(LOG_LEVEL_TOKEN, "WARN"); + String resolved = LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}", Collections.emptyMap(), new Properties(), env); + assertEquals("WARN", resolved); + } + + @Test + public void testPropertyWinsEnvSubstitution() { + Map<String, String> props = new HashMap<>(); + props.put(LOG_LEVEL_TOKEN, "DEBUG"); + Map<String, String> env = new HashMap<>(); + env.put(LOG_LEVEL_TOKEN, "WARN"); + String resolved = LogServiceLogbackXmlImpl.resolveValue("${log.level}", props , new Properties(), env); + assertEquals("DEBUG", resolved); + } + + @Test + public void testRootLogLevel() { + LogServiceLogbackXmlImpl logService = getLogService(); + assertEquals("WARN", logService.getLevel(LogServiceInternal.ROOT_LOGGER).get(LogServiceInternal.ROOT_LOGGER)); + } + + @Test + public void testPropertyLogLevel() { + String logger = "debugPropertyLogger"; + LogServiceLogbackXmlImpl logService = getLogService(); + assertEquals("DEBUG", logService.getLevel(logger).get(logger)); + } + + @Test + public void testSystemPropertyLogLevel() { + String logger = "systemPropertyLogger"; + System.setProperty("LOG_LEVEL", "DEBUG"); + LogServiceLogbackXmlImpl logService = getLogService(); + assertEquals("DEBUG", logService.getLevel(logger).get(logger)); + System.clearProperty("LOG_LEVEL"); + } + + @Test + public void testDefaultValueLogLevel() { + String logger = "defaultValueLogger"; + LogServiceLogbackXmlImpl logService = getLogService(); + assertEquals("DEBUG", logService.getLevel(logger).get(logger)); + } + + @Test + public void testAllLoggerLogLevel() { + LogServiceLogbackXmlImpl logService = getLogService(); + Map<String, String> levels = logService.getLevel(LogServiceInternal.ALL_LOGGER); + assertEquals(4, levels.size()); + assertTrue(levels.containsKey(LogServiceInternal.ROOT_LOGGER)); + } + + private LogServiceLogbackXmlImpl getLogService() { + return new LogServiceLogbackXmlImpl(file); + } + } diff --git a/log/src/test/resources/logback.xml b/log/src/test/resources/logback.xml new file mode 100644 index 0000000000..fe3dfe991f --- /dev/null +++ b/log/src/test/resources/logback.xml @@ -0,0 +1,28 @@ +<?xml version="1.0" encoding="UTF-8" standalone="yes"?> +<!-- + + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<configuration> + <property name="system.log.level" scope="context" value="${LOG_LEVEL}"/> + <property name="debug.log.level" scope="context" value="DEBUG"/> + + <root level="WARN"/> + + <logger name="debugPropertyLogger" level="${debug.log.level}"/> + <logger name="systemPropertyLogger" level="${system.log.level}"/> + <logger name="defaultValueLogger" level="${not.existing.property:-DEBUG}"/> +</configuration> \ No newline at end of file
