From 728c2e6cf500708e0800574e379096fc2498a5c0 Mon Sep 17 00:00:00 2001 From: Andrey Kozel Date: Sat, 22 Jan 2022 00:18:00 +0300 Subject: [PATCH 1/2] Fixes #2489 : Fixed issue related to exceptions thrown from the nested spies. Behaviour of the filtering redundant method calls from stacktrace was changed. Now it removes only lines that have the same method name and line number --- .../creation/bytebuddy/MockMethodAdvice.java | 61 +++++++--------- .../InlineDelegateByteBuddyMockMakerTest.java | 71 +++++++++++++------ 2 files changed, 76 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java index 1490501eae..01ebec07ba 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java @@ -19,6 +19,9 @@ import java.lang.reflect.Method; import java.util.List; import java.util.Map; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; import java.util.concurrent.Callable; import java.util.function.Predicate; @@ -114,27 +117,6 @@ private static void exit( } } - static Throwable hideRecursiveCall(Throwable throwable, int current, Class targetType) { - try { - StackTraceElement[] stack = throwable.getStackTrace(); - int skip = 0; - StackTraceElement next; - do { - next = stack[stack.length - current - ++skip]; - } while (!next.getClassName().equals(targetType.getName())); - int top = stack.length - current - skip; - StackTraceElement[] cleared = new StackTraceElement[stack.length - skip]; - System.arraycopy(stack, 0, cleared, 0, top); - System.arraycopy(stack, top + skip, cleared, top, current); - throwable.setStackTrace(cleared); - return throwable; - } catch (RuntimeException ignored) { - // This should not happen unless someone instrumented or manipulated exception stack - // traces. - return throwable; - } - } - @Override public Callable handle(Object instance, Method origin, Object[] arguments) throws Throwable { MockMethodInterceptor interceptor = interceptors.get(instance); @@ -333,25 +315,32 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen return accessor.invoke(origin, instance, arguments); } catch (InvocationTargetException exception) { Throwable cause = exception.getCause(); - StackTraceElement[] tmpStack = new Throwable().getStackTrace(); - - int skip = tmpStack.length; - // if there is a suitable instance, do not skip the root-cause for the exception - if (instance != null) { - skip = 0; - String causingClassName = instance.getClass().getName(); - StackTraceElement stackFrame; - do { - stackFrame = tmpStack[skip++]; - } while (stackFrame.getClassName().startsWith(causingClassName)); - } - - new ConditionalStackTraceFilter() - .filter(hideRecursiveCall(cause, skip, origin.getDeclaringClass())); + new ConditionalStackTraceFilter().filter(removeRecursiveCalls(cause, origin.getDeclaringClass())); throw cause; } } + static Throwable removeRecursiveCalls(final Throwable cause, final Class declaringClass) { + final List uniqueStackTraceItems = new ArrayList<>(); + final List indexesToBeRemoved = new ArrayList<>(); + for (StackTraceElement element : cause.getStackTrace()) { + final String key = element.getClassName() + element.getLineNumber(); + final int elementIndex = uniqueStackTraceItems.lastIndexOf(key); + uniqueStackTraceItems.add(key); + + if (elementIndex > -1 && declaringClass.getName().equals(element.getClassName())) { + indexesToBeRemoved.add(elementIndex); + } + } + final List adjustedList = new ArrayList<>(Arrays.asList(cause.getStackTrace())); + indexesToBeRemoved.stream() + .sorted(Comparator.reverseOrder()) + .mapToInt(Integer::intValue) + .forEach(adjustedList::remove); + cause.setStackTrace(adjustedList.toArray(new StackTraceElement[]{})); + return cause; + } + private static class ReturnValueWrapper implements Callable { private final Object returned; diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java index e27605640b..c2ceaba152 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java @@ -9,19 +9,14 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; import static org.junit.Assume.assumeTrue; import java.io.IOException; -import java.util.HashMap; -import java.util.List; -import java.util.Observable; -import java.util.Observer; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.concurrent.Callable; import java.util.regex.Pattern; +import java.util.stream.Collectors; import net.bytebuddy.ByteBuddy; import net.bytebuddy.ClassFileVersion; @@ -253,17 +248,44 @@ public void should_leave_causing_stack() throws Exception { mockMaker.createSpy( settings, new MockHandlerImpl<>(settings), new ExceptionThrowingClass()); - StackTraceElement[] returnedStack = null; - try { - proxy.get().throwException(); - } catch (IOException ex) { - returnedStack = ex.getStackTrace(); - } + StackTraceElement[] returnedStack = + assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace(); assertNotNull("Stack trace from mockito expected", returnedStack); - assertEquals(ExceptionThrowingClass.class.getName(), returnedStack[0].getClassName()); - assertEquals("internalThrowException", returnedStack[0].getMethodName()); + List exceptionClassElements = Arrays.stream(returnedStack) + .filter(element -> element.getClassName().equals(ExceptionThrowingClass.class.getName())) + .collect(Collectors.toList()); + assertEquals(3, exceptionClassElements.size()); + assertEquals("internalThrowException", exceptionClassElements.get(0).getMethodName()); + assertEquals("internalThrowException", exceptionClassElements.get(1).getMethodName()); + assertEquals("throwException", exceptionClassElements.get(2).getMethodName()); + } + + @Test + public void should_leave_causing_stack_with_two_spies() throws Exception { + //given + MockSettingsImpl settingsEx = new MockSettingsImpl<>(); + settingsEx.setTypeToMock(ExceptionThrowingClass.class); + settingsEx.defaultAnswer(Answers.CALLS_REAL_METHODS); + Optional proxyEx = + mockMaker.createSpy(settingsEx, new MockHandlerImpl<>(settingsEx), new ExceptionThrowingClass()); + + MockSettingsImpl settingsWr = new MockSettingsImpl<>(); + settingsWr.setTypeToMock(WrapperClass.class); + settingsWr.defaultAnswer(Answers.CALLS_REAL_METHODS); + Optional proxyWr = + mockMaker.createSpy(settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass()); + + //when + IOException ex = assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get())); + List wrapperClassElements = Arrays.stream(ex.getStackTrace()) + .filter(element -> element.getClassName().equals(WrapperClass.class.getName())) + .collect(Collectors.toList()); + + //then + assertEquals(1, wrapperClassElements.size()); + assertEquals("callWrapped", wrapperClassElements.get(0).getMethodName()); } @Test @@ -271,30 +293,33 @@ public void should_remove_recursive_self_call_from_stack_trace() throws Exceptio StackTraceElement[] stack = new StackTraceElement[] { new StackTraceElement("foo", "", "", -1), - new StackTraceElement(SampleInterface.class.getName(), "", "", -1), + new StackTraceElement(SampleInterface.class.getName(), "", "", 15), new StackTraceElement("qux", "", "", -1), new StackTraceElement("bar", "", "", -1), + new StackTraceElement(SampleInterface.class.getName(), "", "", 15), new StackTraceElement("baz", "", "", -1) }; Throwable throwable = new Throwable(); throwable.setStackTrace(stack); - throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class); + throwable = MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class); assertThat(throwable.getStackTrace()) .isEqualTo( new StackTraceElement[] { new StackTraceElement("foo", "", "", -1), + new StackTraceElement("qux", "", "", -1), new StackTraceElement("bar", "", "", -1), + new StackTraceElement(SampleInterface.class.getName(), "", "", 15), new StackTraceElement("baz", "", "", -1) }); } @Test - public void should_handle_missing_or_inconsistent_stack_trace() throws Exception { + public void should_handle_missing_or_inconsistent_stack_trace() { Throwable throwable = new Throwable(); throwable.setStackTrace(new StackTraceElement[0]); - assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class)) + assertThat(MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class)) .isSameAs(throwable); } @@ -579,6 +604,12 @@ public T value() { } } + public static class WrapperClass { + public void callWrapped(ExceptionThrowingClass exceptionThrowingClass) throws IOException { + exceptionThrowingClass.throwException(); + } + } + public static class GenericSubClass extends GenericClass {} public static class ExceptionThrowingClass { From 7ec800d5d15ec0fc35f3f55ac0fe69ff0002c89a Mon Sep 17 00:00:00 2001 From: Andrey Kozel Date: Sat, 22 Jan 2022 10:57:31 +0300 Subject: [PATCH 2/2] Fixes #2489 : Fixed codestyle --- .../creation/bytebuddy/MockMethodAdvice.java | 14 ++++--- .../InlineDelegateByteBuddyMockMakerTest.java | 38 ++++++++++++------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java index 01ebec07ba..580f22b309 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java @@ -315,7 +315,8 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen return accessor.invoke(origin, instance, arguments); } catch (InvocationTargetException exception) { Throwable cause = exception.getCause(); - new ConditionalStackTraceFilter().filter(removeRecursiveCalls(cause, origin.getDeclaringClass())); + new ConditionalStackTraceFilter() + .filter(removeRecursiveCalls(cause, origin.getDeclaringClass())); throw cause; } } @@ -332,12 +333,13 @@ static Throwable removeRecursiveCalls(final Throwable cause, final Class decl indexesToBeRemoved.add(elementIndex); } } - final List adjustedList = new ArrayList<>(Arrays.asList(cause.getStackTrace())); + final List adjustedList = + new ArrayList<>(Arrays.asList(cause.getStackTrace())); indexesToBeRemoved.stream() - .sorted(Comparator.reverseOrder()) - .mapToInt(Integer::intValue) - .forEach(adjustedList::remove); - cause.setStackTrace(adjustedList.toArray(new StackTraceElement[]{})); + .sorted(Comparator.reverseOrder()) + .mapToInt(Integer::intValue) + .forEach(adjustedList::remove); + cause.setStackTrace(adjustedList.toArray(new StackTraceElement[] {})); return cause; } diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java index c2ceaba152..3d42f3a377 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java @@ -249,13 +249,17 @@ public void should_leave_causing_stack() throws Exception { settings, new MockHandlerImpl<>(settings), new ExceptionThrowingClass()); StackTraceElement[] returnedStack = - assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace(); + assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace(); assertNotNull("Stack trace from mockito expected", returnedStack); - List exceptionClassElements = Arrays.stream(returnedStack) - .filter(element -> element.getClassName().equals(ExceptionThrowingClass.class.getName())) - .collect(Collectors.toList()); + List exceptionClassElements = + Arrays.stream(returnedStack) + .filter( + element -> + element.getClassName() + .equals(ExceptionThrowingClass.class.getName())) + .collect(Collectors.toList()); assertEquals(3, exceptionClassElements.size()); assertEquals("internalThrowException", exceptionClassElements.get(0).getMethodName()); assertEquals("internalThrowException", exceptionClassElements.get(1).getMethodName()); @@ -264,26 +268,34 @@ public void should_leave_causing_stack() throws Exception { @Test public void should_leave_causing_stack_with_two_spies() throws Exception { - //given + // given MockSettingsImpl settingsEx = new MockSettingsImpl<>(); settingsEx.setTypeToMock(ExceptionThrowingClass.class); settingsEx.defaultAnswer(Answers.CALLS_REAL_METHODS); Optional proxyEx = - mockMaker.createSpy(settingsEx, new MockHandlerImpl<>(settingsEx), new ExceptionThrowingClass()); + mockMaker.createSpy( + settingsEx, + new MockHandlerImpl<>(settingsEx), + new ExceptionThrowingClass()); MockSettingsImpl settingsWr = new MockSettingsImpl<>(); settingsWr.setTypeToMock(WrapperClass.class); settingsWr.defaultAnswer(Answers.CALLS_REAL_METHODS); Optional proxyWr = - mockMaker.createSpy(settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass()); + mockMaker.createSpy( + settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass()); - //when - IOException ex = assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get())); - List wrapperClassElements = Arrays.stream(ex.getStackTrace()) - .filter(element -> element.getClassName().equals(WrapperClass.class.getName())) - .collect(Collectors.toList()); + // when + IOException ex = + assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get())); + List wrapperClassElements = + Arrays.stream(ex.getStackTrace()) + .filter( + element -> + element.getClassName().equals(WrapperClass.class.getName())) + .collect(Collectors.toList()); - //then + // then assertEquals(1, wrapperClassElements.size()); assertEquals("callWrapped", wrapperClassElements.get(0).getMethodName()); }