[GitHub] chetanmeh closed pull request #3: SLING-7529 - implemented layout inheritance in log encoder

[GitBox <gi...@apache.org>]

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


With regards,
Apache Git Services