Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate on bytes or chars #88

Merged
merged 7 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import edu.umd.cs.findbugs.annotations.NonNull;

import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.function.BiConsumer;
import java.util.function.BinaryOperator;
Expand Down Expand Up @@ -62,18 +63,46 @@ private List<String> getChunks() {
}

/**
* Builds the string such that it does not exceed maxSize, including the truncation string.
* Builds the string such that it does not exceed maxSize in bytes, including the truncation string.
*
* @param maxSize the maximum size of the resultant string.
* @return A string comprising as many of the joined chunks that will fit in the given size, plus the truncation
* string if truncation was necessary.
* @deprecated use the explicit {@link #buildByBytes(int)} or {@link #buildByChars(int)} method according to your requirements.
*/
@Deprecated
public String build(final int maxSize) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we deprecate this one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think we should.

And instead of passing the chunkOnChars boolean field, would it be better if we expose two methods directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure; preference for names? buildByBytes/buildByChars?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with that.

return build(maxSize, false);
}

/**
* Builds the string such that it does not exceed maxSize in bytes, including the truncation string.
*
* @param maxSize the maximum size of the resultant string.
* @return A string comprising as many of the joined chunks that will fit in the given size, plus the truncation
* string if truncation was necessary.
*/
public String buildByBytes(final int maxSize) {
return build(maxSize, false);
}

/**
* Builds the string such that it does not exceed maxSize in chars, including the truncation string.
*
* @param maxSize the maximum size of the resultant string.
* @return A string comprising as many of the joined chunks that will fit in the given size, plus the truncation
* string if truncation was necessary.
*/
public String buildByChars(final int maxSize) {
return build(maxSize, true);
}

private String build(final int maxSize, final boolean chunkOnChars) {
List<String> parts = getChunks();
if (truncateStart) {
Collections.reverse(parts);
}
List<String> truncatedParts = parts.stream().collect(new Joiner(truncationText, maxSize));
List<String> truncatedParts = parts.stream().collect(new Joiner(truncationText, maxSize, chunkOnChars));
if (truncateStart) {
Collections.reverse(truncatedParts);
}
Expand Down Expand Up @@ -148,13 +177,19 @@ private static class Joiner implements Collector<String, Joiner.Accumulator, Lis

private final int maxLength;
private final String truncationText;
private final boolean chunkOnChars;

Joiner(final String truncationText, final int maxLength) {
if (maxLength < truncationText.length()) {
throw new IllegalArgumentException("Maximum length is less than truncation text.");
}
Joiner(final String truncationText, final int maxLength, final boolean chunkOnChars) {
this.truncationText = truncationText;
this.maxLength = maxLength;
this.chunkOnChars = chunkOnChars;
if (maxLength < getLength(truncationText)) {
throw new IllegalArgumentException("Maximum length is less than truncation text.");
}
}

private int getLength(final String text) {
return chunkOnChars ? text.length() : text.getBytes(StandardCharsets.UTF_8).length;
}

@Override
Expand Down Expand Up @@ -196,17 +231,17 @@ void add(final String chunk) {
if (truncated) {
return;
}
if (length + chunk.length() > maxLength) {
if (length + getLength(chunk) > maxLength) {
truncated = true;
return;
}
chunks.add(chunk);
length += chunk.length();
length += getLength(chunk);
}

List<String> truncate() {
if (truncated) {
if (length + truncationText.length() > maxLength) {
if (length + getLength(truncationText) > maxLength) {
chunks.remove(chunks.size() - 1);
}
chunks.add(truncationText);
Expand Down
114 changes: 78 additions & 36 deletions src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package io.jenkins.plugins.checks.api;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.nio.charset.StandardCharsets;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

Expand All @@ -27,96 +30,135 @@ public class TruncatedStringTest {
@Parameterized.Parameter(1)
public boolean chunkOnNewlines;

/**
* Parameter for chunking on chars (or not!).
*/
@Parameterized.Parameter(2)
public boolean chunkOnChars;

@Parameterized.Parameters(name = "{0}")
public static Object[][] parameters() {
return new Object[][]{
{"Chunks", false},
{"Newlines", true}
{"Chunks+Bytes", false, false},
{"Newlines+Bytes", true, false},
{"Chunks+Chars", false, true},
{"Newlines+Chars", true, true}
};
}

private TruncatedString.Builder getBuilder() {
TruncatedString.Builder builder = new TruncatedString.Builder()
private TruncatedString.Builder builder;

@Before
public void makeBuilder() {
this.builder = new TruncatedString.Builder()
.withTruncationText(MESSAGE);
if (chunkOnNewlines) {
return builder.setChunkOnNewlines();
builder.setChunkOnNewlines();
}
return builder;
}

private String build(final int maxSize) {
return chunkOnChars ? builder.build().buildByChars(maxSize) : builder.build().buildByBytes(maxSize);
}

private String buildRawString() {
return builder.build().toString();
}

@Test
public void shouldBuildStrings() {
TruncatedString.Builder builder = getBuilder();
builder.addText("Hello\n");
assertThat(builder.build()).asString().isEqualTo("Hello\n");
assertThat(builder.build().build(1000)).isEqualTo("Hello\n");
assertThat(buildRawString()).isEqualTo("Hello\n");
assertThat(build(1000)).isEqualTo("Hello\n");
builder.addText(", world!");
assertThat(builder.build()).asString().isEqualTo("Hello\n, world!");
assertThat(builder.build().build(1000)).isEqualTo("Hello\n, world!");
assertThat(buildRawString()).isEqualTo("Hello\n, world!");
assertThat(build(1000)).isEqualTo("Hello\n, world!");
}

@Test
public void shouldTruncateStrings() {
TruncatedString.Builder builder = getBuilder();
builder.addText("xxxxxxxxx\n"); // 10
assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\n");
assertThat(build(20)).isEqualTo("xxxxxxxxx\n");
builder.addText("yyyy\n"); // 5, doesn't cause overflow
assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nyyyy\n");
assertThat(build(20)).isEqualTo("xxxxxxxxx\nyyyy\n");
builder.addText("zzzzzz\n"); // 7, does cause overflow
assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nTruncated");
assertThat(build(20)).isEqualTo("xxxxxxxxx\nTruncated");
}

@Test
public void shouldHandleEdgeCases() {
TruncatedString.Builder builder = getBuilder();
assertThat(builder.build().build(10)).isEqualTo("");
assertThat(builder.build()).asString().isEqualTo("");
assertThat(build(10)).isEqualTo("");
assertThat(buildRawString()).isEqualTo("");
builder.addText("xxxxxxxxxxxxxx\n"); // 15
assertThat(builder.build().build(10)).isEqualTo("Truncated");
assertThat(build(10)).isEqualTo("Truncated");
assertThatThrownBy(() -> {
builder.build().build(5);
build(5);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessage("Maximum length is less than truncation text.");
}

@Test
public void shouldHandleReversedChunking() {
TruncatedString.Builder builder = getBuilder()
.setTruncateStart();
builder.setTruncateStart();
builder.addText("zzzz\n"); // 5
assertThat(builder.build().build(20)).isEqualTo("zzzz\n");
assertThat(build(20)).isEqualTo("zzzz\n");
builder.addText("xxxx\n"); // 5, doesn't cause overflow
assertThat(builder.build().build(20)).isEqualTo("zzzz\nxxxx\n");
assertThat(build(20)).isEqualTo("zzzz\nxxxx\n");
builder.addText("cccc\n"); // 5, doesn't cause overflow
assertThat(builder.build().build(20)).isEqualTo("zzzz\nxxxx\ncccc\n");
assertThat(build(20)).isEqualTo("zzzz\nxxxx\ncccc\n");
builder.addText("aaaaaa\n"); // 7, does cause overflow
assertThat(builder.build().build(20)).isEqualTo("Truncatedcccc\naaaaaa\n");
assertThat(build(20)).isEqualTo("Truncatedcccc\naaaaaa\n");
}

@Test
public void shouldHandleEdgeCasesReversed() {
TruncatedString.Builder builder = getBuilder()
.setTruncateStart();
assertThat(builder.build().build(10)).isEqualTo("");
assertThat(builder.build()).asString().isEqualTo("");
builder.setTruncateStart();
assertThat(build(10)).isEqualTo("");
assertThat(buildRawString()).isEqualTo("");
builder.addText("xxxxxxxxxxxxxx\n"); // 15
assertThat(builder.build().build(10)).isEqualTo("Truncated");
assertThat(build(10)).isEqualTo("Truncated");
assertThatThrownBy(() -> {
builder.build().build(5);
build(5);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessage("Maximum length is less than truncation text.");
}

@Test
public void shouldChunkNewlinesDifferently() {
TruncatedString.Builder builder = getBuilder();
builder.addText("xxxxxxxxxx"); // 10
builder.addText("yyyyyyyyyyy"); // 11
assertThat(builder.build().build(20)).isEqualTo(chunkOnNewlines ? "Truncated" : "xxxxxxxxxxTruncated");
assertThat(build(20)).isEqualTo(chunkOnNewlines ? "Truncated" : "xxxxxxxxxxTruncated");

builder = getBuilder();
makeBuilder();
builder.addText("wwww\n"); // 5
builder.addText("xxxx\nyyyy\nzzzzz\n"); // 16
assertThat(builder.build().build(20)).isEqualTo(chunkOnNewlines ? "wwww\nxxxx\nTruncated" : "wwww\nTruncated");
assertThat(build(20)).isEqualTo(chunkOnNewlines ? "wwww\nxxxx\nTruncated" : "wwww\nTruncated");
}

@Test
public void shouldTruncateByBytesOrChars() {
builder.addText("☃☃☃\n"); // 3 + 1
assertThat(buildRawString().length()).isEqualTo(4);
assertThat(buildRawString().getBytes(StandardCharsets.UTF_8).length).isEqualTo(10);
assertThat(build(20)).isEqualTo("☃☃☃\n");

builder.addText("🕴️🕴️\n"); // 2 + 1
assertThat(buildRawString().length()).isEqualTo(11);
assertThat(buildRawString().getBytes(StandardCharsets.UTF_8).length).isEqualTo(25);
assertThat(build(20)).isEqualTo(chunkOnChars ? "☃☃☃\n🕴️🕴️\n" : "☃☃☃\nTruncated");
}

@Test
public void shouldHandleLongCharsInTruncationText() {
String truncationText = "E_TOO_MUCH_☃";
assertThat(truncationText.length()).isEqualTo(12);
assertThat(truncationText.getBytes(StandardCharsets.UTF_8).length).isEqualTo(14);

builder.withTruncationText(truncationText);
builder.addText("xxxx\n"); // 5
builder.addText("x\n"); // 2
assertThat(build(20)).isEqualTo("xxxx\nx\n");
builder.addText("xxxxxxxxxxxxxxx"); // 15
assertThat(build(20)).isEqualTo(chunkOnChars ? "xxxx\nx\nE_TOO_MUCH_☃" : "xxxx\nE_TOO_MUCH_☃");
}
}