You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2023/01/30 19:54:32 UTC

[logging-log4j2] 01/06: PR #704 from Carter but with fixes in StackLocator getCurrentStackTrace().

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 6c8577a578d19b65e435d118deb9823f9f1ed659
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jan 17 18:31:42 2022 -0500

    PR #704 from Carter but with fixes in StackLocator
    getCurrentStackTrace().
    
    - Call push() one by one instead of addAll() to match log4j-api.
    - Do not allocate an extra collection.
    - Update the test.
    - Constants should be in upper case.
---
 .../logging/log4j/util/StackLocatorUtilTest.java   | 76 +++++++++++++++++++---
 .../util/PrivateSecurityManagerStackTraceUtil.java |  5 +-
 2 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StackLocatorUtilTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StackLocatorUtilTest.java
index 8fe11c3f8e..15ff9b0735 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StackLocatorUtilTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StackLocatorUtilTest.java
@@ -16,14 +16,16 @@
  */
 package org.apache.logging.log4j.util;
 
-import java.util.ArrayDeque;
 import java.util.Deque;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.engine.execution.InterceptingExecutableInvoker;
 import org.junit.jupiter.engine.execution.InvocationInterceptorChain;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 public class StackLocatorUtilTest {
 
@@ -75,15 +77,11 @@ public class StackLocatorUtilTest {
     @Test
     public void testGetCurrentStackTrace() throws Exception {
         final Deque<Class<?>> classes = StackLocatorUtil.getCurrentStackTrace();
-        final Deque<Class<?>> reversed = new ArrayDeque<>(classes.size());
-        while (!classes.isEmpty()) {
-            reversed.push(classes.pop());
+        while (classes.peekFirst() != StackLocatorUtil.class) {
+            classes.removeFirst();
         }
-        while (reversed.peek() != StackLocatorUtil.class) {
-            reversed.pop();
-        }
-        reversed.pop(); // ReflectionUtil
-        assertSame(StackLocatorUtilTest.class, reversed.pop());
+        classes.removeFirst(); // StackLocatorUtil
+        assertSame(StackLocatorUtilTest.class, classes.removeFirst());
     }
 
     @Test
@@ -112,4 +110,62 @@ public class StackLocatorUtilTest {
         assertEquals(this.getClass(), clazz, "Incorrect class");
     }
 
+    private final class Foo {
+
+        private StackTraceElement foo() {
+            return new Bar().bar(); // <--- testCalcLocation() line
+        }
+
+    }
+
+    private final class Bar {
+
+        private StackTraceElement bar() {
+            return baz();
+        }
+
+        private StackTraceElement baz() {
+            return quux();
+        }
+
+    }
+
+    private StackTraceElement quux() {
+        final StackLocator stackLocator = StackLocator.getInstance();
+        return stackLocator.calcLocation("org.apache.logging.log4j.util.StackLocatorUtilTest$Bar");
+    }
+
+    @Test
+    public void testCalcLocation() {
+        /*
+         * We are setting up a stack trace that looks like:
+         *  - org.apache.logging.log4j.util.test.StackLocatorTest#quux(line:118)
+         *  - org.apache.logging.log4j.util.test.StackLocatorTest$Bar#baz(line:112)
+         *  - org.apache.logging.log4j.util.test.StackLocatorTest$Bar#bar(line:108)
+         *  - org.apache.logging.log4j.util.test.StackLocatorTest$Foo(line:100)
+         *
+         * We are pretending that org.apache.logging.log4j.util.test.StackLocatorTest$Bar is the logging class, and
+         * org.apache.logging.log4j.util.test.StackLocatorTest$Foo is where the log line emanated.
+         */
+        final StackTraceElement element = new Foo().foo();
+        assertEquals("org.apache.logging.log4j.util.StackLocatorUtilTest$Foo", element.getClassName());
+        // The line number below may need adjustment if this file is changed.
+        assertEquals(116, element.getLineNumber());
+    }
+
+    @Test
+    public void testCalcLocationWhenNotInTheStack() {
+        final StackLocator stackLocator = StackLocator.getInstance();
+        final StackTraceElement stackTraceElement = stackLocator.calcLocation("java.util.Logger");
+        assertNull(stackTraceElement);
+    }
+
+    static class ClassLocator {
+
+        public Class<?> locateClass() {
+            final StackLocator stackLocator = StackLocator.getInstance();
+            return stackLocator.getCallerClass(ClassLocator.class);
+        }
+    }
+
 }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java
index 2f2be07986..94ca4ff555 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/PrivateSecurityManagerStackTraceUtil.java
@@ -17,6 +17,7 @@
 package org.apache.logging.log4j.util;
 
 import java.util.ArrayDeque;
+import java.util.Collections;
 import java.util.Deque;
 
 /**
@@ -64,9 +65,7 @@ final class PrivateSecurityManagerStackTraceUtil {
     static Deque<Class<?>> getCurrentStackTrace() {
         final Class<?>[] array = SECURITY_MANAGER.getClassContext();
         final Deque<Class<?>> classes = new ArrayDeque<>(array.length);
-        for (final Class<?> clazz : array) {
-            classes.push(clazz);
-        }
+        Collections.addAll(classes, array);
         return classes;
     }