diff --git a/src/main/java/org/cryptomator/common/ErrorCode.java b/src/main/java/org/cryptomator/common/ErrorCode.java index 51fb355b6..7def1287b 100644 --- a/src/main/java/org/cryptomator/common/ErrorCode.java +++ b/src/main/java/org/cryptomator/common/ErrorCode.java @@ -1,5 +1,6 @@ package org.cryptomator.common; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.base.Throwables; @@ -80,7 +81,7 @@ public class ErrorCode { if (causalChain.size() > 1) { var rootCause = causalChain.get(causalChain.size() - 1); var parentOfRootCause = causalChain.get(causalChain.size() - 2); - var rootSpecificFrames = nonOverlappingFrames(parentOfRootCause.getStackTrace(), rootCause.getStackTrace()); + var rootSpecificFrames = countTopmostFrames(rootCause.getStackTrace(), parentOfRootCause.getStackTrace()); return new ErrorCode(throwable, rootCause, rootSpecificFrames); } else { return new ErrorCode(throwable, throwable, ALL_FRAMES); @@ -107,11 +108,31 @@ public class ErrorCode { return result; } - private static int nonOverlappingFrames(StackTraceElement[] frames, StackTraceElement[] enclosingFrames) { - // Compute the number of elements in `frames` not contained in `enclosingFrames` by iterating backwards - // Result should usually be equal to the difference in size of both traces - var i = reverseStream(enclosingFrames).iterator(); - return (int) reverseStream(frames).dropWhile(f -> i.hasNext() && i.next().equals(f)).count(); + /** + * Counts the number of additional frames contained in allFrames but not in bottomFrames. + *

+ * If allFrames does not end with bottomFrames, it is considered distinct and all its frames are counted. + * + * @param allFrames Some stack frames + * @param bottomFrames Other stack frames, potentially forming the bottom of the stack of allFrames + * @return The number of additional frames in allFrames. In most cases this should be equal to the difference in size. + */ + // visible for testing + static int countTopmostFrames(StackTraceElement[] allFrames, StackTraceElement[] bottomFrames) { + if (allFrames.length < bottomFrames.length) { + // if frames had been stacked on top of bottomFrames, allFrames would be larger + return allFrames.length; + } else { + return allFrames.length - commonSuffixLength(allFrames, bottomFrames); + } + } + + // visible for testing + static int commonSuffixLength(T[] set, T[] subset) { + Preconditions.checkArgument(set.length >= subset.length); + // iterate items backwards as long as they are identical + var iterator = reverseStream(subset).iterator(); + return (int) reverseStream(set).takeWhile(item -> iterator.hasNext() && iterator.next().equals(item)).count(); } private static Stream reverseStream(T[] array) { diff --git a/src/test/java/org/cryptomator/common/ErrorCodeTest.java b/src/test/java/org/cryptomator/common/ErrorCodeTest.java index 34c0c2ec0..dea5c6ba1 100644 --- a/src/test/java/org/cryptomator/common/ErrorCodeTest.java +++ b/src/test/java/org/cryptomator/common/ErrorCodeTest.java @@ -1,59 +1,119 @@ package org.cryptomator.common; +import com.google.common.base.CharMatcher; +import com.google.common.base.Splitter; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.converter.ConvertWith; +import org.junit.jupiter.params.converter.SimpleArgumentConverter; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; public class ErrorCodeTest { - private static ErrorCode codeCaughtFrom(RunnableThrowingException runnable) { - try { - runnable.run(); - throw new IllegalStateException("should not reach this point"); - } catch (RuntimeException e) { - return ErrorCode.of(e); - } - } + private final StackTraceElement foo = new StackTraceElement("ErrorCodeTest", "foo", null, 0); + private final StackTraceElement bar = new StackTraceElement("ErrorCodeTest", "bar", null, 0); + private final StackTraceElement baz = new StackTraceElement("ErrorCodeTest", "baz", null, 0); + private final Exception fooException = Mockito.mock(NullPointerException.class, "fooException"); @Test @DisplayName("same exception leads to same error code") - public void testDifferentErrorCodes() { - var code1 = codeCaughtFrom(this::throwNpe); - var code2 = codeCaughtFrom(this::throwNpe); + public void testDeterministicErrorCode() { + Mockito.doReturn(new StackTraceElement[]{foo, bar, baz}).when(fooException).getStackTrace(); + var code1 = ErrorCode.of(fooException); + var code2 = ErrorCode.of(fooException); Assertions.assertEquals(code1.toString(), code2.toString()); } - private void throwNpe() { - throwException(new NullPointerException()); + @Test + @DisplayName("three error code segments change independently") + public void testErrorCodeSegments() { + Exception fooBarException = Mockito.mock(IndexOutOfBoundsException.class, "fooBarException"); + Mockito.doReturn(new StackTraceElement[]{foo, foo, foo}).when(fooBarException).getStackTrace(); + Mockito.doReturn(fooException).when(fooBarException).getCause(); + Mockito.doReturn(new StackTraceElement[]{bar, bar, bar, foo, foo, foo}).when(fooException).getStackTrace(); + + var code = ErrorCode.of(fooBarException); + + Assertions.assertNotEquals(code.throwableCode(), code.rootCauseCode()); + Assertions.assertNotEquals(code.rootCauseCode(), code.methodCode()); } - private void throwException(RuntimeException e) throws RuntimeException { - throw e; + @DisplayName("commonSuffixLength()") + @ParameterizedTest + @CsvSource({"1 2 3, 1 2 3, 3", "1 2 3, 0 2 3, 2", "1 2 3 4, 3 4, 2", "1 2 3 4, 5 6, 0", "1 2 3 4 5 6,, 0",}) + public void commonSuffixLength1(@ConvertWith(IntegerArrayConverter.class) Integer[] set, @ConvertWith(IntegerArrayConverter.class) Integer[] subset, int expected) { + var result = ErrorCode.commonSuffixLength(set, subset); + + Assertions.assertEquals(expected, result); } - @DisplayName("when different cause but same root cause") + @DisplayName("commonSuffixLength() with too short array") + @ParameterizedTest + @CsvSource({"1 2, 3 4 5 6", ",1 2 3 4 5 6",}) + public void commonSuffixLength2(@ConvertWith(IntegerArrayConverter.class) Integer[] set, @ConvertWith(IntegerArrayConverter.class) Integer[] subset) { + Assertions.assertThrows(IllegalArgumentException.class, () -> { + ErrorCode.commonSuffixLength(set, subset); + }); + } + + @Test + @DisplayName("countTopmostFrames() with partially overlapping suffix") + public void testCountTopmostFrames1() { + var allFrames = new StackTraceElement[]{foo, bar, baz, bar, foo}; + var bottomFrames = new StackTraceElement[]{baz, bar, foo}; + + int result = ErrorCode.countTopmostFrames(allFrames, bottomFrames); + + Assertions.assertEquals(2, result); + } + + @Test + @DisplayName("countTopmostFrames() without overlapping suffix") + public void testCountTopmostFrames2() { + var allFrames = new StackTraceElement[]{foo, foo, foo}; + var bottomFrames = new StackTraceElement[]{bar, bar, bar}; + + int result = ErrorCode.countTopmostFrames(allFrames, bottomFrames); + + Assertions.assertEquals(3, result); + } + + @Test + @DisplayName("countUniqueFrames() fully overlapping") + public void testCountUniqueFrames3() { + var allFrames = new StackTraceElement[]{foo, bar, baz}; + var bottomFrames = new StackTraceElement[]{foo, bar, baz}; + + int result = ErrorCode.countTopmostFrames(allFrames, bottomFrames); + + Assertions.assertEquals(0, result); + } + + @DisplayName("when different exception with same root cause") @Nested - public class SameRootCauseDifferentCause { + public class DifferentExceptionWithSameRootCause { - private final ErrorCode code1 = codeCaughtFrom(this::foo); - private final ErrorCode code2 = codeCaughtFrom(this::bar); + private final Exception fooBarException = Mockito.mock(IllegalArgumentException.class, "fooBarException"); + private final Exception fooBazException = Mockito.mock(IndexOutOfBoundsException.class, "fooBazException"); - private void foo() throws IllegalArgumentException { - try { - throwNpe(); - } catch (NullPointerException e) { - throw new IllegalArgumentException(e); - } - } + private ErrorCode code1; + private ErrorCode code2; - private void bar() throws IllegalStateException { - try { - throwNpe(); - } catch (NullPointerException e) { - throw new IllegalStateException(e); - } + @BeforeEach + private void setup() { + Mockito.doReturn(new StackTraceElement[]{baz, bar, foo}).when(fooException).getStackTrace(); + Mockito.doReturn(new StackTraceElement[]{foo}).when(fooBarException).getStackTrace(); + Mockito.doReturn(new StackTraceElement[]{foo}).when(fooBazException).getStackTrace(); + Mockito.doReturn(fooException).when(fooBarException).getCause(); + Mockito.doReturn(fooException).when(fooBazException).getCause(); + this.code1 = ErrorCode.of(fooBarException); + this.code2 = ErrorCode.of(fooBazException); } @Test @@ -82,23 +142,21 @@ public class ErrorCodeTest { } - @DisplayName("when same cause but different call stack") + @DisplayName("when same exception with different call stacks") @Nested - public class SameCauseDifferentCallStack { + public class SameExceptionDifferentCallStack { - private final ErrorCode code1 = codeCaughtFrom(this::foo); - private final ErrorCode code2 = codeCaughtFrom(this::bar); + private final Exception barException = Mockito.mock(NullPointerException.class, "barException"); - private void foo() throws NullPointerException { - try { - throwNpe(); - } catch (NullPointerException e) { - throw new IllegalArgumentException(e); - } - } + private ErrorCode code1; + private ErrorCode code2; - private void bar() throws NullPointerException { - foo(); + @BeforeEach + private void setup() { + Mockito.doReturn(new StackTraceElement[]{foo, bar, baz}).when(fooException).getStackTrace(); + Mockito.doReturn(new StackTraceElement[]{foo, baz, bar}).when(barException).getStackTrace(); + this.code1 = ErrorCode.of(fooException); + this.code2 = ErrorCode.of(barException); } @Test @@ -114,9 +172,9 @@ public class ErrorCodeTest { } @Test - @DisplayName("rootCauseCodes are equal") + @DisplayName("rootCauseCodes are different") public void testSameRootCauseCodes() { - Assertions.assertEquals(code1.rootCauseCode(), code2.rootCauseCode()); + Assertions.assertNotEquals(code1.rootCauseCode(), code2.rootCauseCode()); } @Test @@ -127,4 +185,19 @@ public class ErrorCodeTest { } + public static class IntegerArrayConverter extends SimpleArgumentConverter { + + @Override + protected Integer[] convert(Object source, Class targetType) { + if (source == null) { + return new Integer[0]; + } else if (source instanceof String s && Integer[].class.isAssignableFrom(targetType)) { + return Splitter.on(CharMatcher.inRange('0', '9').negate()).splitToStream(s).map(Integer::valueOf).toArray(Integer[]::new); + } else { + throw new IllegalArgumentException("Conversion from " + source.getClass() + " to " + targetType + " not supported."); + } + } + + } + } \ No newline at end of file