fixed second part of error code (#1741)

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 <em>additional</em> frames contained in <code>allFrames</code> but not in <code>bottomFrames</code>.
+	 * <p>
+	 * If <code>allFrames</code> does not end with <code>bottomFrames</code>, 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 <code>allFrames</code>
+	 * @return The number of additional frames in <code>allFrames</code>. 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 <T> 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 <T> Stream<T> reverseStream(T[] array) {

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<RuntimeException> 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")
-	@Nested
-	public class SameRootCauseDifferentCause {
+	@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};
 
-		private final ErrorCode code1 = codeCaughtFrom(this::foo);
-		private final ErrorCode code2 = codeCaughtFrom(this::bar);
+		int result = ErrorCode.countTopmostFrames(allFrames, bottomFrames);
 
-		private void foo() throws IllegalArgumentException {
-			try {
-				throwNpe();
-			} catch (NullPointerException e) {
-				throw new IllegalArgumentException(e);
-			}
-		}
+		Assertions.assertEquals(2, result);
+	}
 
-		private void bar() throws IllegalStateException {
-			try {
-				throwNpe();
-			} catch (NullPointerException e) {
-				throw new IllegalStateException(e);
-			}
+	@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 DifferentExceptionWithSameRootCause {
+
+		private final Exception fooBarException = Mockito.mock(IllegalArgumentException.class, "fooBarException");
+		private final Exception fooBazException = Mockito.mock(IndexOutOfBoundsException.class, "fooBazException");
+
+		private ErrorCode code1;
+		private ErrorCode code2;
+
+		@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.");
+			}
+		}
+
+	}
+
 }