Refactor and add tests for optimized assembly traceback (#3949)

Follow-up to #3901:
* Renamed `Substring` to `StackLineView`
* Implemented tests for `StackLineView`
* Corrected `contains` and `startsWith` implementations

Author: chemicL

reactor-core/src/main/java/reactor/core/publisher/Traces.java

@@ -120,14 +120,14 @@ static boolean isUserCode(String line) {
 	 * from the assembly stack trace
 	 */
 	static String[] extractOperatorAssemblyInformationParts(String source) {
-		Iterator<Substring> traces = trimmedNonemptyLines(source);
+		Iterator<StackLineView> traces = trimmedNonemptyLines(source);
 
 		if (!traces.hasNext()) {
 			return new String[0];
 		}
 
-		Substring prevLine = null;
-		Substring currentLine = traces.next();
+		StackLineView prevLine = null;
+		StackLineView currentLine = traces.next();
 
 		if (currentLine.isUserCode()) {
 			// No line is a Reactor API line.
@@ -157,20 +157,20 @@ static String[] extractOperatorAssemblyInformationParts(String source) {
 	 *
 	 * @implNote This implementation attempts to minimize allocations.
 	 */
-	private static Iterator<Substring> trimmedNonemptyLines(String source) {
-		return new Iterator<Substring>() {
-			private int index = 0;
+	private static Iterator<StackLineView> trimmedNonemptyLines(String source) {
+		return new Iterator<StackLineView>() {
+			private int           index = 0;
 			@Nullable
-			private Substring next = getNextLine();
+			private StackLineView next  = getNextLine();
 
 			@Override
 			public boolean hasNext() {
 				return next != null;
 			}
 
 			@Override
-			public Substring next() {
-				Substring current = next;
+			public StackLineView next() {
+				StackLineView current = next;
 				if (current == null) {
 					throw new NoSuchElementException();
 				}
@@ -179,13 +179,13 @@ public Substring next() {
 			}
 
 			@Nullable
-			private Substring getNextLine() {
+			private StackLineView getNextLine() {
 				while (index < source.length()) {
 					int end = source.indexOf('\n', index);
 					if (end == -1) {
 						end = source.length();
 					}
-					Substring line = new Substring(source, index, end).trim();
+					StackLineView line = new StackLineView(source, index, end).trim();
 					index = end + 1;
 					if (!line.isEmpty()) {
 						return line;
@@ -196,63 +196,70 @@ private Substring getNextLine() {
 		};
 	}
 
-	// XXX: Explain.
-	private static final class Substring {
-		private final String str;
-		private final int start;
-		private final int end;
+	/**
+	 * Provides optimized access to underlying {@link String} with common operations to
+	 * view the stack trace line without unnecessary allocation of temporary
+	 * {@code String} objects.
+	 */
+	static final class StackLineView {
+
+		private final String underlying;
+		private final int    start;
+		private final int    end;
 
-		Substring(String str, int start, int end) {
-			this.str = str;
+		StackLineView(String underlying, int start, int end) {
+			this.underlying = underlying;
 			this.start = start;
 			this.end = end;
 		}
 
-		Substring trim() {
+		StackLineView trim() {
 			int newStart = start;
-			while (newStart < end && str.charAt(newStart) <= ' ') {
+			while (newStart < end && underlying.charAt(newStart) <= ' ') {
 				newStart++;
 			}
 			int newEnd = end;
-			while (newEnd > newStart && str.charAt(newEnd - 1) <= ' ') {
+			while (newEnd > newStart && underlying.charAt(newEnd - 1) <= ' ') {
 				newEnd--;
 			}
-			return newStart == start && newEnd == end ? this : new Substring(str, newStart, newEnd);
+			return newStart == start && newEnd == end ? this : new StackLineView(
+					underlying, newStart, newEnd);
 		}
 
 		boolean isEmpty() {
 			return start == end;
 		}
 
 		boolean startsWith(String prefix) {
-			return str.startsWith(prefix, start);
+			boolean canFit = end - start >= prefix.length();
+			return canFit && underlying.startsWith(prefix, start);
 		}
 
 		boolean contains(String substring) {
-			int index = str.indexOf(substring, start);
-			return index >= 0 && index < end;
+			int index = underlying.indexOf(substring, start);
+			return index >= start && (index + (substring.length() - 1) < end);
 		}
 
 		boolean isUserCode() {
 			return !startsWith(PUBLISHER_PACKAGE_PREFIX) || contains("Test");
 		}
 
-		Substring withoutLocationSuffix() {
-			int linePartIndex = str.indexOf('(', start);
+		StackLineView withoutLocationSuffix() {
+			int linePartIndex = underlying.indexOf('(', start);
 			return linePartIndex > 0 && linePartIndex < end
-				? new Substring(str, start, linePartIndex)
+				? new StackLineView(underlying, start, linePartIndex)
 				: this;
 		}
 
-		Substring withoutPublisherPackagePrefix() {
+		StackLineView withoutPublisherPackagePrefix() {
 			return startsWith(PUBLISHER_PACKAGE_PREFIX)
-				? new Substring(str, start + PUBLISHER_PACKAGE_PREFIX.length(), end)
+				? new StackLineView(underlying, start + PUBLISHER_PACKAGE_PREFIX.length(), end)
 				: this;
 		}
 
 		@Override
 		public String toString() {
-			return str.substring(start, end);
+			return underlying.substring(start, end);
 		}
 	}
 }

reactor-core/src/test/java/reactor/core/publisher/TracesTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2021 VMware Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2018-2024 VMware Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -146,4 +146,79 @@ public void shouldSanitizeTrue() {
 		assertThat(Traces.shouldSanitize("java.lang.reflect")).isTrue();
 	}
 
+	@Test
+	void stackLineViewSanityTest() {
+		String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";
+
+		int end = stackLine.indexOf('\n');
+		if (end == -1) {
+			System.out.println("No end-of-line");
+			end = stackLine.length();
+		}
+
+		Traces.StackLineView view = new Traces.StackLineView(stackLine, 0, end)
+				.trim();
+
+		assertThat(view.toString()).isEqualTo(stackLine.trim());
+		assertThat(view.isEmpty()).isFalse();
+		assertThat(view.isUserCode()).isFalse();
+		assertThat(view.contains("Flux.filter")).isTrue();
+		assertThat(view.startsWith("reactor.core.publisher.Flux")).isTrue();
+	}
+
+	@Test
+	void stackLineViewLimitsAreCheckedAtStart() {
+		String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";
+
+		Traces.StackLineView incompleteView =
+				new Traces.StackLineView(stackLine, stackLine.length() / 2, stackLine.length())
+						.trim();
+
+		assertThat(incompleteView.toString()).isEqualTo("ux.filter(Flux.java:4209)");
+		assertThat(incompleteView.contains(".filter")).isTrue();
+		assertThat(incompleteView.contains("ux.f")).isTrue();
+		assertThat(incompleteView.contains("lux.f")).isFalse();
+		assertThat(incompleteView.contains("09)")).isTrue();
+		assertThat(incompleteView.startsWith("ux.")).isTrue();
+		assertThat(incompleteView.startsWith("lux.")).isFalse();
+		assertThat(incompleteView.startsWith("ux.filter(Flux.java:4209)")).isTrue();
+		assertThat(incompleteView.startsWith("lux.filter(Flux.java:4209)")).isFalse();
+	}
+
+	@Test
+	void stackLineViewLimitsAreCheckedAtEnd() {
+		String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";
+
+		Traces.StackLineView incompleteView =
+				new Traces.StackLineView(stackLine, 0, stackLine.length() / 2)
+						.trim();
+
+		assertThat(incompleteView.toString()).isEqualTo("reactor.core.publisher.Fl");
+		assertThat(incompleteView.contains("Fl")).isTrue();
+		assertThat(incompleteView.contains("Flu")).isFalse();
+		assertThat(incompleteView.startsWith("reactor.core.publisher.Fl")).isTrue();
+		assertThat(incompleteView.startsWith("reactor.core.publisher.Flux")).isFalse();
+	}
+
+	@Test
+	void stackLineViewLocationSuffixGetsRemoved() {
+		String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";
+
+		Traces.StackLineView view =
+				new Traces.StackLineView(stackLine, 0, stackLine.length()).trim();
+
+		assertThat(view.withoutLocationSuffix()
+		               .toString()).isEqualTo("reactor.core.publisher.Flux.filter");
+	}
+
+	@Test
+	void stackLineViewPublisherPackagePrefixGetsRemoved() {
+		String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";
+
+		Traces.StackLineView view =
+				new Traces.StackLineView(stackLine, 0, stackLine.length()).trim();
+
+		assertThat(view.withoutPublisherPackagePrefix()
+		               .toString()).isEqualTo("Flux.filter(Flux.java:4209)");
+	}
 }