[
https://issues.apache.org/jira/browse/SLING-7529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16394969#comment-16394969
]
ASF GitHub Bot commented on SLING-7529:
---------------------------------------
chetanmeh closed pull request #3: SLING-7529 - implemented layout inheritance
in log encoder
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/3
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/src/main/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoder.java
b/src/main/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoder.java
index 702acf8..996a5b0 100644
---
a/src/main/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoder.java
+++
b/src/main/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoder.java
@@ -45,12 +45,24 @@ public LoggerSpecificEncoder(Layout<ILoggingEvent>
defaultLayout) {
}
private Layout<ILoggingEvent> getLayout(String loggerName) {
- // TODO Handle layout inheritance wrt logger names
- Layout<ILoggingEvent> layout = layoutByCategory.get(loggerName);
- if (layout == null) {
- layout = defaultLayout;
+ String bestMatchLayoutKey = getBestMatchLayoutKey(loggerName);
+ return layoutByCategory.getOrDefault(bestMatchLayoutKey,
defaultLayout);
+ }
+
+ private String getBestMatchLayoutKey(String loggerName) {
+ if (layoutByCategory.containsKey(loggerName)) {
+ // fastpath for exact name match
+ return loggerName;
+ }
+ String bestMatch = loggerName;
+ int bestMatchLength = 0;
+ for (String layoutKey : layoutByCategory.keySet()) {
+ if (loggerName.startsWith(layoutKey) &&
loggerName.charAt(layoutKey.length()) == '.' && layoutKey.length() >
bestMatchLength) {
+ bestMatch = layoutKey;
+ bestMatchLength = layoutKey.length();
+ }
}
- return layout;
+ return bestMatch;
}
private byte[] convertToBytes(String s) {
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/internal/util/AbstractPatternLayoutWrapper.java
b/src/test/java/org/apache/sling/commons/log/logback/internal/util/AbstractPatternLayoutWrapper.java
new file mode 100644
index 0000000..93ed24f
--- /dev/null
+++
b/src/test/java/org/apache/sling/commons/log/logback/internal/util/AbstractPatternLayoutWrapper.java
@@ -0,0 +1,177 @@
+/*
+ * 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.
+ */
+
+package org.apache.sling.commons.log.logback.internal.util;
+
+import ch.qos.logback.classic.PatternLayout;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Context;
+import ch.qos.logback.core.pattern.PostCompileProcessor;
+import ch.qos.logback.core.status.Status;
+
+import java.util.Map;
+
+/**
+ * Abstract wrapper for {@link PatternLayout} class. Can be extended to
implement 'Decorator' design pattern.
+ *
+ * @apiNote This has probably no use outside of testing
+ */
+public abstract class AbstractPatternLayoutWrapper extends PatternLayout {
+
+ protected final PatternLayout wrapped;
+
+ protected AbstractPatternLayoutWrapper(final PatternLayout wrapped) {
+ this.wrapped = wrapped;
+ }
+
+ @Override
+ public String doLayout(final ILoggingEvent event) {
+ return wrapped.doLayout(event);
+ }
+
+ @Override
+ public String getFileHeader() {
+ return wrapped.getFileHeader();
+ }
+
+ @Override
+ public String getPresentationHeader() {
+ return wrapped.getPresentationHeader();
+ }
+
+ @Override
+ public String getPresentationFooter() {
+ return wrapped.getPresentationFooter();
+ }
+
+ @Override
+ public String getFileFooter() {
+ return wrapped.getFileFooter();
+ }
+
+ @Override
+ public String getContentType() {
+ return wrapped.getContentType();
+ }
+
+ @Override
+ public void setContext(final Context context) {
+ wrapped.setContext(context);
+ }
+
+ @Override
+ public Context getContext() {
+ return wrapped.getContext();
+ }
+
+ @Override
+ public void addStatus(final Status status) {
+ wrapped.addStatus(status);
+ }
+
+ @Override
+ public void addInfo(final String s) {
+ wrapped.addInfo(s);
+ }
+
+ @Override
+ public void addInfo(final String msg, final Throwable ex) {
+ wrapped.addInfo(msg, ex);
+ }
+
+ @Override
+ public void addWarn(final String s) {
+ wrapped.addWarn(s);
+ }
+
+ @Override
+ public void addWarn(final String msg, final Throwable ex) {
+ wrapped.addWarn(msg, ex);
+ }
+
+ @Override
+ public void addError(final String s) {
+ wrapped.addError(s);
+ }
+
+ @Override
+ public void addError(final String msg, final Throwable ex) {
+ wrapped.addError(msg, ex);
+ }
+
+ @Override
+ public void start() {
+ wrapped.start();
+ }
+
+ @Override
+ public void stop() {
+ wrapped.stop();
+ }
+
+ @Override
+ public boolean isStarted() {
+ return wrapped.isStarted();
+ }
+
+ @Override
+ public Map<String, String> getDefaultConverterMap() {
+ return wrapped.getDefaultConverterMap();
+ }
+
+ @Override
+ public Map<String, String> getEffectiveConverterMap() {
+ return wrapped.getEffectiveConverterMap();
+ }
+
+ @Override
+ public void setPostCompileProcessor(PostCompileProcessor<ILoggingEvent>
postCompileProcessor) {
+ wrapped.setPostCompileProcessor(postCompileProcessor);
+ }
+
+ @Override
+ public String getPattern() {
+ return wrapped.getPattern();
+ }
+
+ @Override
+ public void setPattern(String pattern) {
+ wrapped.setPattern(pattern);
+ }
+
+ @Override
+ public String toString() {
+ return wrapped.toString();
+ }
+
+ @Override
+ public Map<String, String> getInstanceConverterMap() {
+ return wrapped.getInstanceConverterMap();
+ }
+
+ @Override
+ public boolean isOutputPatternAsHeader() {
+ return wrapped.isOutputPatternAsHeader();
+ }
+
+ @Override
+ public void setOutputPatternAsHeader(boolean outputPatternAsHeader) {
+ wrapped.isOutputPatternAsHeader();
+ }
+}
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoderTest.java
b/src/test/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoderTest.java
new file mode 100644
index 0000000..a32e406
--- /dev/null
+++
b/src/test/java/org/apache/sling/commons/log/logback/internal/util/LoggerSpecificEncoderTest.java
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.sling.commons.log.logback.internal.util;
+
+import ch.qos.logback.classic.PatternLayout;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import org.apache.felix.framework.util.ImmutableList;
+import org.apache.sling.commons.log.logback.internal.LogConfig;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.HashSet;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class LoggerSpecificEncoderTest {
+
+ private LoggerSpecificEncoder tested;
+
+ @Mock
+ private ILoggingEvent mockTestEvent;
+
+ @Before
+ public void setUp() {
+ tested = new LoggerSpecificEncoder(new PrefixTestLayout("DEFAULT:"));
+ when(mockTestEvent.getMessage()).thenReturn("test message");
+
when(mockTestEvent.getLoggerName()).thenReturn("org.apache.sling.testing.FooBar");
+ }
+
+ @Test
+ public void testShouldReturnWithDefaultLayoutForNoConfigs() {
+ assertThat(tested.encode(mockTestEvent), is(equalTo("DEFAULT:test
message".getBytes())));
+ }
+
+ @Test
+ public void testShouldIgnoreNonmatchingLayoutCategories() {
+ LogConfig logConfigMock = mock(LogConfig.class);
+ when(logConfigMock.getCategories()).thenReturn(new
HashSet<>(ImmutableList.newInstance("org.apache.commons",
"com.initech.sling")));
+ when(logConfigMock.createLayout()).thenReturn(new
PrefixTestLayout("INITECH:"));
+ tested.addLogConfig(logConfigMock);
+
+ assertThat(tested.encode(mockTestEvent), is(equalTo("DEFAULT:test
message".getBytes())));
+ }
+
+ @Test
+ public void testShouldIgnorePartialMatchingPackageName() {
+ LogConfig logConfigMock = mock(LogConfig.class);
+ when(logConfigMock.getCategories()).thenReturn(new
HashSet<>(ImmutableList.newInstance("org.apache.sling.test")));
+ when(logConfigMock.createLayout()).thenReturn(new
PrefixTestLayout("INITECH:"));
+ tested.addLogConfig(logConfigMock);
+
+ assertThat(tested.encode(mockTestEvent), is(equalTo("DEFAULT:test
message".getBytes())));
+ }
+
+ @Test
+ public void testShouldUseExactMatchingCategoryLayout() {
+ LogConfig logConfigMock = mock(LogConfig.class);
+ when(logConfigMock.getCategories()).thenReturn(new
HashSet<>(ImmutableList.newInstance("org.apache.sling.testing.FooBar")));
+ when(logConfigMock.createLayout()).thenReturn(new
PrefixTestLayout("INITECH:"));
+ tested.addLogConfig(logConfigMock);
+
+ assertThat(tested.encode(mockTestEvent), is(equalTo("INITECH:test
message".getBytes())));
+ }
+
+ @Test
+ public void testShouldUseInheritedCategoryLayout() {
+ LogConfig logConfigMock = mock(LogConfig.class);
+ when(logConfigMock.getCategories()).thenReturn(new
HashSet<>(ImmutableList.newInstance("org.apache")));
+ when(logConfigMock.createLayout()).thenReturn(new
PrefixTestLayout("INITECH:"));
+ tested.addLogConfig(logConfigMock);
+
+ assertThat(tested.encode(mockTestEvent), is(equalTo("INITECH:test
message".getBytes())));
+ }
+
+ /**
+ * Simple partial implementation of {@link PatternLayout} that redirects
all method calls that are not explicitly extended to an
+ * underlying mock (available as a protected {@link
PrefixTestLayout#wrapped} field).
+ */
+ private static class PrefixTestLayout extends AbstractPatternLayoutWrapper
{
+
+ private final String prefix;
+
+ private PrefixTestLayout(String prefix) {
+ super(mock(PatternLayout.class));
+ this.prefix = prefix;
+ }
+
+ @Override
+ public String doLayout(ILoggingEvent event) {
+ return prefix + event.getMessage();
+ }
+ }
+}
\ No newline at end of file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Log message layouts are not property inherited
> ----------------------------------------------
>
> Key: SLING-7529
> URL: https://issues.apache.org/jira/browse/SLING-7529
> Project: Sling
> Issue Type: Bug
> Components: Commons
> Affects Versions: Commons Log 5.1.2
> Reporter: Fryderyk Wysocki
> Assignee: Chetan Mehrotra
> Priority: Major
>
> Steps to reproduce on AEM 6.3 with Apache Sling Commons Log 5.1.2:
> 1. Create Logger Configuration for Logger "my.project", log file "error.log",
> log level 'Information" and message pattern "My project: \{5}"
> 2. Create an Slf4J logger for class my.project.Sample and use it to infolog
> message "test foo"
> 3. Change the configuration created in step 1 - update logger to
> "my.project.Sample"
> 4. Use the logger created in step 2 to infolog message "test bar"
> Expected result:
> 5. error.log file would contain messages "My project: test foo" and "My
> project: test bar"
> Actual result:
> 5. error.log file contains the message "test foo" in the format configured as
> the default format, but also contains the message "My project: test bar" -
> the second message, in the appropriate format.
> Investigation:
> Checking the source code, I've discovered the following in class
> 'LoggerSpecificEncoder', starting from line 47:
> {code:java}
> private Layout<ILoggingEvent> getLayout(String loggerName) {
> // TODO Handle layout inheritance wrt logger names
> Layout<ILoggingEvent> layout = layoutByCategory.get(loggerName);
> if (layout == null) {
> layout = defaultLayout;
> }
> return layout;
> }
> {code}
> Which means that, unless the configuration is created for the exact logger
> name, the default one will be used - the functionality is not broken, but
> simply not implemented.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
