sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SLING-7529) Log message layouts are not property inherited
Date Mon, 12 Mar 2018 09:03:00 GMT

    [ 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:
users@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)

Mime
View raw message