You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/05/09 16:13:52 UTC

logging-log4j2 git commit: LOG4J2-1382 Copying a MutableLogEvent using Log4jLogEvent.Builder should not unnecessarily obtain caller location information.

Repository: logging-log4j2
Updated Branches:
  refs/heads/master 1e32cafd7 -> d380220d2


LOG4J2-1382 Copying a MutableLogEvent using Log4jLogEvent.Builder should not unnecessarily obtain caller location information.


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/d380220d
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/d380220d
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/d380220d

Branch: refs/heads/master
Commit: d380220d2c219207eab3a70ff8768337c888a101
Parents: 1e32caf
Author: rpopma <rp...@apache.org>
Authored: Tue May 10 01:14:17 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Tue May 10 01:14:17 2016 +0900

----------------------------------------------------------------------
 .../logging/log4j/core/impl/Log4jLogEvent.java  |  7 +-
 .../log4j/core/impl/MutableLogEvent.java        | 30 +++++++++
 .../log4j/core/impl/Log4jLogEventTest.java      | 68 ++++++++++++++++++++
 src/changes/changes.xml                         |  3 +
 4 files changed, 106 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/d380220d/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java
index 7c9bcbf..1ba20c9 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java
@@ -98,8 +98,11 @@ public class Log4jLogEvent implements LogEvent {
         public Builder(LogEvent other) {
             Objects.requireNonNull(other);
             if (other instanceof RingBufferLogEvent) {
-                RingBufferLogEvent evt = (RingBufferLogEvent) other;
-                evt.initializeBuilder(this);
+                ((RingBufferLogEvent) other).initializeBuilder(this);
+                return;
+            }
+            if (other instanceof MutableLogEvent) {
+                ((MutableLogEvent) other).initializeBuilder(this);
                 return;
             }
             this.loggerFqcn = other.getLoggerFqcn();

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/d380220d/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
index be4ba24..02e67a7 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
@@ -162,6 +162,9 @@ public class MutableLogEvent implements LogEvent, ReusableMessage {
 
     @Override
     public Level getLevel() {
+        if (level == null) {
+            level = Level.OFF; // LOG4J2-462, LOG4J2-465
+        }
         return level;
     }
 
@@ -426,4 +429,31 @@ public class MutableLogEvent implements LogEvent, ReusableMessage {
         return Log4jLogEvent.deserialize(Log4jLogEvent.serialize(this, includeLocation));
     }
 
+    /**
+     * Initializes the specified {@code Log4jLogEvent.Builder} from this {@code MutableLogEvent}.
+     * @param builder the builder whose fields to populate
+     */
+    public void initializeBuilder(Log4jLogEvent.Builder builder) {
+        builder.setContextMap(contextMap) //
+                .setContextStack(contextStack) //
+                .setEndOfBatch(endOfBatch) //
+                .setIncludeLocation(includeLocation) //
+                .setLevel(getLevel()) // ensure non-null
+                .setLoggerFqcn(loggerFqcn) //
+                .setLoggerName(loggerName) //
+                .setMarker(marker) //
+                .setMessage(getNonNullImmutableMessage()) // ensure non-null & immutable
+                .setNanoTime(nanoTime) //
+                .setSource(source) //
+                .setThreadId(threadId) //
+                .setThreadName(threadName) //
+                .setThreadPriority(threadPriority) //
+                .setThrown(getThrown()) // may deserialize from thrownProxy
+                .setThrownProxy(thrownProxy) // avoid unnecessarily creating thrownProxy
+                .setTimeMillis(timeMillis);
+    }
+
+    private Message getNonNullImmutableMessage() {
+        return message != null ? message : new SimpleMessage(String.valueOf(messageText));
+    }
 }

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/d380220d/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java
index 6388b15..df4adfb 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java
@@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.lang.reflect.Field;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -284,6 +285,73 @@ public class Log4jLogEventTest {
     }
 
     @Test
+    public void testBuilderCorrectlyCopiesMutableLogEvent() throws Exception {
+        final Map<String, String> contextMap = new HashMap<>();
+        contextMap.put("A", "B");
+        final ContextStack contextStack = ThreadContext.getImmutableStack();
+        final Exception exception = new Exception("test");
+        final Marker marker = MarkerManager.getMarker("EVENTTEST");
+        final Message message = new SimpleMessage("foo");
+        final StackTraceElement stackTraceElement = new StackTraceElement("A", "B", "file", 123);
+        final String fqcn = "qualified";
+        final String name = "Ceci n'est pas une pipe";
+        final String threadName = "threadName";
+        final MutableLogEvent event = new MutableLogEvent();
+        event.setContextMap(contextMap);
+        event.setContextStack(contextStack);
+        event.setEndOfBatch(true);
+        event.setIncludeLocation(true);
+        //event.setSource(stackTraceElement); // cannot be explicitly set
+        event.setLevel(Level.FATAL);
+        event.setLoggerFqcn(fqcn);
+        event.setLoggerName(name);
+        event.setMarker(marker);
+        event.setMessage(message);
+        event.setNanoTime(1234567890L);
+        event.setThreadName(threadName);
+        event.setThrown(exception);
+        event.setTimeMillis(987654321L);
+
+        assertSame(contextMap, event.getContextMap());
+        assertSame(contextStack, event.getContextStack());
+        assertEquals(true, event.isEndOfBatch());
+        assertEquals(true, event.isIncludeLocation());
+        assertSame(Level.FATAL, event.getLevel());
+        assertSame(fqcn, event.getLoggerFqcn());
+        assertSame(name, event.getLoggerName());
+        assertSame(marker, event.getMarker());
+        assertSame(message, event.getMessage());
+        assertEquals(1234567890L, event.getNanoTime());
+        //assertSame(stackTraceElement, event.getSource()); // don't invoke
+        assertSame(threadName, event.getThreadName());
+        assertSame(exception, event.getThrown());
+        assertEquals(987654321L, event.getTimeMillis());
+
+        LogEvent e2 = new Log4jLogEvent.Builder(event).build();
+        assertSame(contextMap, e2.getContextMap());
+        assertSame(contextStack, e2.getContextStack());
+        assertEquals(true, e2.isEndOfBatch());
+        assertEquals(true, e2.isIncludeLocation());
+        assertSame(Level.FATAL, e2.getLevel());
+        assertSame(fqcn, e2.getLoggerFqcn());
+        assertSame(name, e2.getLoggerName());
+        assertSame(marker, e2.getMarker());
+        assertSame(message, e2.getMessage());
+        assertEquals(1234567890L, e2.getNanoTime());
+        //assertSame(stackTraceElement, e2.getSource()); // don't invoke
+        assertSame(threadName, e2.getThreadName());
+        assertSame(exception, e2.getThrown());
+        assertEquals(987654321L, e2.getTimeMillis());
+
+        // use reflection to get value of source field in log event copy:
+        // invoking the getSource() method would initialize the field
+        Field fieldSource = Log4jLogEvent.class.getDeclaredField("source");
+        fieldSource.setAccessible(true);
+        Object value = fieldSource.get(e2);
+        assertNull("source in copy", value);
+    }
+
+    @Test
     public void testEquals() {
         final Map<String, String> contextMap = new HashMap<>();
         contextMap.put("A", "B");

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/d380220d/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6700840..daa4b24 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -24,6 +24,9 @@
   </properties>
   <body>
     <release version="2.6" date="2016-MM-DD" description="GA Release 2.6">
+      <action issue="LOG4J2-1382" dev="rpopma" type="fix">
+        Copying a MutableLogEvent using Log4jLogEvent.Builder should not unnecessarily obtain caller location information.
+      </action>
       <action issue="LOG4J2-1011" dev="mikes" type="add">
         Document dependencies for layouts.
       </action>