From 46ff97c56427605f9d0d0fbec512305b4ff98514 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Fri, 1 Jan 2021 13:16:02 +0000 Subject: [PATCH 1/5] Allow for moving truncation into the implementing API --- pom.xml | 2 +- .../plugins/checks/api/ChecksOutput.java | 74 ++++++++-- .../plugins/checks/api/TruncatedString.java | 132 ++++++++++++++++++ 3 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java diff --git a/pom.xml b/pom.xml index 8056e07d..4c0618ec 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ 8 - 1.2.1 + 1.3.0 -SNAPSHOT diff --git a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java index addd96bf..e6100fda 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java +++ b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java @@ -1,11 +1,13 @@ package io.jenkins.plugins.checks.api; +import org.apache.commons.lang3.StringUtils; + import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; -import static java.util.Objects.*; +import static java.util.Objects.requireNonNull; /** * An output of a check. The output usually contains the most useful information like summary, description, @@ -13,12 +15,12 @@ */ public class ChecksOutput { private final String title; - private final String summary; - private final String text; + private final TruncatedString summary; + private final TruncatedString text; private final List annotations; private final List images; - private ChecksOutput(final String title, final String summary, final String text, + private ChecksOutput(final String title, final TruncatedString summary, final TruncatedString text, final List annotations, final List images) { this.title = title; this.summary = summary; @@ -34,7 +36,9 @@ private ChecksOutput(final String title, final String summary, final String text * the source to copy from */ public ChecksOutput(final ChecksOutput that) { - this(that.getTitle().orElse(null), that.getSummary().orElse(null), that.getText().orElse(null), + this(that.getTitle().orElse(null), + that.getSummary().map(TruncatedString::fromString).orElse(null), + that.getText().map(TruncatedString::fromString).orElse(null), that.getChecksAnnotations(), that.getChecksImages()); } @@ -43,11 +47,31 @@ public Optional getTitle() { } public Optional getSummary() { - return Optional.ofNullable(summary); + return Optional.ofNullable(summary).map(TruncatedString::build); + } + + /** + * Get the output summary, truncated by {@link TruncatedString} to maxSize. + * + * @param maxSize maximum size to truncate summary to. + * @return Summary, truncated to maxSize with truncation message if appropriate. + */ + public Optional getSummary(final int maxSize) { + return Optional.ofNullable(summary).flatMap(s -> Optional.ofNullable(s.build(maxSize))); } public Optional getText() { - return Optional.ofNullable(text); + return Optional.ofNullable(text).map(TruncatedString::build); + } + + /** + * Get the output text, truncated by {@link TruncatedString} to maxSize. + * + * @param maxSize maximum size to truncate text to. + * @return Text, truncated to maxSize with truncation message if appropriate. + */ + public Optional getText(final int maxSize) { + return Optional.ofNullable(text).flatMap(s -> Optional.ofNullable(s.build(maxSize))); } public List getChecksAnnotations() { @@ -74,8 +98,8 @@ public String toString() { */ public static class ChecksOutputBuilder { private String title; - private String summary; - private String text; + private TruncatedString summary; + private TruncatedString text; private List annotations; private List images; @@ -114,6 +138,22 @@ public ChecksOutputBuilder withTitle(final String title) { */ @SuppressWarnings("HiddenField") // builder pattern public ChecksOutputBuilder withSummary(final String summary) { + return withSummary(new TruncatedString.Builder().addText(summary).build()); + } + + /** + * Sets the summary of the check run, using a {@link TruncatedString}. + * + *

+ * Note that for the GitHub check runs, the {@code summary} supports Markdown. + *

+ * + * @param summary + * the summary of the check run as a {@link TruncatedString} + * @return this builder + */ + @SuppressWarnings("HiddenField") + public ChecksOutputBuilder withSummary(final TruncatedString summary) { this.summary = requireNonNull(summary); return this; } @@ -131,6 +171,22 @@ public ChecksOutputBuilder withSummary(final String summary) { */ @SuppressWarnings("HiddenField") // builder pattern public ChecksOutputBuilder withText(final String text) { + return withText(new TruncatedString.Builder().addText(text).build()); + } + + /** + * Adds the details description for a check run, using a {@link TruncatedString}. This parameter supports Markdown. + * + *

+ * Note that for a GitHub check run, the {@code text} supports Markdown. + *

+ * + * @param text + * the details description in Markdown as a {@link TruncatedString} + * @return this builder + */ + @SuppressWarnings("HiddenField") + public ChecksOutputBuilder withText(final TruncatedString text) { this.text = requireNonNull(text); return this; } diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java new file mode 100644 index 00000000..71e6681b --- /dev/null +++ b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java @@ -0,0 +1,132 @@ +package io.jenkins.plugins.checks.api; + +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Utility wrapper that silently truncates output with a message at a certain size. + * + * The GitHub Checks API has a size limit on text fields. Because it also accepts markdown, it is not trivial to + * truncate to the required length as this could lead to unterminated syntax. The use of this class allows for adding + * chunks of complete markdown until an overflow is detected, at which point a message will be added and all future + * additions will be silently discarded. + */ +public class TruncatedString { + + @NonNull + private final List chunks; + @NonNull + private final String truncationText; + + /** + * Create a {@link TruncatedString} with the provided chunks and truncation message. + * + * @param truncationText the message to be appended should maxSize be exceeded, e.g. + * "Some output is not shown here, see more on [Jenkins](url)." + * @param chunks a list of {@link CharSequence}s that are to be concatenated. + */ + private TruncatedString(@NonNull final String truncationText, @NonNull final List chunks) { + this.truncationText = Objects.requireNonNull(truncationText); + this.chunks = Objects.requireNonNull(chunks); + } + + /** + * Wrap the provided string as a {@link TruncatedString}. + * + * @param string String to wrap as a {@link TruncatedString} + * @return a {@link TruncatedString} wrapping the provided input + */ + static TruncatedString fromString(final String string) { + return new TruncatedString.Builder().addText(string).build(); + } + + @Override + public String toString() { + return String.join("", chunks); + } + + /** + * Builds the string without truncation. + * + * @return A string comprising the joined chunks. + */ + @CheckForNull + public String build() { + return chunks.isEmpty() ? null : String.join("", chunks); + } + + /** + * Builds the string such that it does not exceed maxSize, 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. + */ + @CheckForNull + public String build(final int maxSize) { + if (chunks.isEmpty()) { + return null; + } + String quickJoin = String.join("", chunks); + if (quickJoin.length() <= maxSize) { + return quickJoin; + } + StringBuilder builder = new StringBuilder(); + for (CharSequence chunk: chunks) { + if (builder.length() + chunk.length() + truncationText.length() < maxSize) { + builder.append(chunk); + } + else { + builder.append(truncationText); + break; + } + } + return builder.toString(); + } + + /** + * Builder for {@link TruncatedString}. + */ + public static class Builder { + private String truncationText = "Output truncated."; + private final List chunks = new ArrayList<>(); + + /** + * Builds the {@link TruncatedString}. + * + * @return the build {@link TruncatedString}. + */ + public TruncatedString build() { + return new TruncatedString(truncationText, chunks); + } + + /** + * Sets the truncation text. + * + * @param truncationText the text to append on overflow + * @return this builder + */ + @SuppressWarnings("HiddenField") + public Builder withTruncationText(@NonNull final String truncationText) { + this.truncationText = Objects.requireNonNull(truncationText); + return this; + } + + /** + * Adds a chunk of text to the buidler. + * + * @param chunk the chunk of text to append to this builder + * @return this buidler + */ + public Builder addText(@NonNull final CharSequence chunk) { + this.chunks.add(Objects.requireNonNull(chunk)); + return this; + } + + } + +} From 034e114c4346c6ec8137869a45ef7ee431765d38 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Fri, 8 Jan 2021 14:39:27 +0000 Subject: [PATCH 2/5] More options for truncation --- .../plugins/checks/api/ChecksOutput.java | 10 +- .../checks/api/ChunkedTruncatedString.java | 63 +++++++ .../checks/api/NewlineTruncatedString.java | 63 +++++++ .../plugins/checks/api/TruncatedString.java | 174 ++++++++++++------ .../api/ChunkedTruncatedStringTest.java | 79 ++++++++ .../api/NewlineTruncatedStringTest.java | 78 ++++++++ 6 files changed, 404 insertions(+), 63 deletions(-) create mode 100644 src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java create mode 100644 src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java create mode 100644 src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java create mode 100644 src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java diff --git a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java index e6100fda..9f075e3e 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java +++ b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java @@ -1,7 +1,5 @@ package io.jenkins.plugins.checks.api; -import org.apache.commons.lang3.StringUtils; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -47,7 +45,7 @@ public Optional getTitle() { } public Optional getSummary() { - return Optional.ofNullable(summary).map(TruncatedString::build); + return Optional.ofNullable(summary).map(TruncatedString::toString); } /** @@ -61,7 +59,7 @@ public Optional getSummary(final int maxSize) { } public Optional getText() { - return Optional.ofNullable(text).map(TruncatedString::build); + return Optional.ofNullable(text).map(TruncatedString::toString); } /** @@ -138,7 +136,7 @@ public ChecksOutputBuilder withTitle(final String title) { */ @SuppressWarnings("HiddenField") // builder pattern public ChecksOutputBuilder withSummary(final String summary) { - return withSummary(new TruncatedString.Builder().addText(summary).build()); + return withSummary(TruncatedString.fromString(summary)); } /** @@ -171,7 +169,7 @@ public ChecksOutputBuilder withSummary(final TruncatedString summary) { */ @SuppressWarnings("HiddenField") // builder pattern public ChecksOutputBuilder withText(final String text) { - return withText(new TruncatedString.Builder().addText(text).build()); + return withText(TruncatedString.fromString(text)); } /** diff --git a/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java new file mode 100644 index 00000000..748c0304 --- /dev/null +++ b/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java @@ -0,0 +1,63 @@ +package io.jenkins.plugins.checks.api; + +import edu.umd.cs.findbugs.annotations.NonNull; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * Implementation of {@link TruncatedString} that truncates strings to the nearest supplied chunk. This allows for + * ensuring that, for example, markdown syntax is not truncated midway through a ``` block, or a table, that would + * result in poor formatting or confusing output. + */ +@SuppressWarnings("PMD.MissingStaticMethodInNonInstantiatableClass") +public class ChunkedTruncatedString extends TruncatedString { + + @NonNull + private final List chunks; + + private ChunkedTruncatedString(@NonNull final String truncationText, final boolean truncateStart, @NonNull final List chunks) { + super(truncationText, truncateStart); + this.chunks = Collections.unmodifiableList(chunks); + } + + @Override + public String toString() { + return String.join("", chunks); + } + + @Override + protected List getChunks() { + return new ArrayList<>(chunks); + } + + /** + * Builder for {@link ChunkedTruncatedString}. + */ + public static class Builder extends TruncatedString.Builder { + private final List chunks = new ArrayList<>(); + + @Override + protected Builder self() { + return this; + } + + @Override + public TruncatedString build() { + return new ChunkedTruncatedString(getTruncationText(), isTruncateStart(), chunks); + } + + /** + * Adds a chunk of text to the buidler. + * + * @param chunk the chunk of text to append to this builder + * @return this buidler + */ + public Builder addText(@NonNull final String chunk) { + this.chunks.add(Objects.requireNonNull(chunk)); + return this; + } + } +} diff --git a/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java new file mode 100644 index 00000000..cada6479 --- /dev/null +++ b/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java @@ -0,0 +1,63 @@ +package io.jenkins.plugins.checks.api; + +import edu.umd.cs.findbugs.annotations.NonNull; + +import java.util.Arrays; +import java.util.List; + +/** + * Implementation of {@link TruncatedString} that truncates strings to the nearest newline. + */ +@SuppressWarnings("PMD.MissingStaticMethodInNonInstantiatableClass") +public class NewlineTruncatedString extends TruncatedString { + + @NonNull + private final String string; + + private NewlineTruncatedString(@NonNull final String truncationText, final boolean truncateStart, @NonNull final String string) { + super(truncationText, truncateStart); + this.string = string; + } + + @Override + public String toString() { + return string; + } + + @Override + protected List getChunks() { + return Arrays.asList(string.split("(?<=\r?\n)")); + } + + /** + * Builder for {@link NewlineTruncatedString}. + */ + public static class Builder extends TruncatedString.Builder { + + private String string = ""; + + @Override + protected Builder self() { + return this; + } + + @Override + public TruncatedString build() { + return new NewlineTruncatedString(getTruncationText(), isTruncateStart(), string); + } + + /** + * Set the string to truncate. + * + * @param string the string to truncate + * @return this builder + */ + @SuppressWarnings("HiddenField") + public Builder withString(final String string) { + this.string = string; + return this; + } + + } + +} diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java index 71e6681b..4d6aebc7 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java +++ b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java @@ -3,35 +3,30 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import java.util.*; +import java.util.function.BiConsumer; +import java.util.function.BinaryOperator; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collector; /** * Utility wrapper that silently truncates output with a message at a certain size. - * + *

* The GitHub Checks API has a size limit on text fields. Because it also accepts markdown, it is not trivial to * truncate to the required length as this could lead to unterminated syntax. The use of this class allows for adding * chunks of complete markdown until an overflow is detected, at which point a message will be added and all future * additions will be silently discarded. */ -public class TruncatedString { +public abstract class TruncatedString { - @NonNull - private final List chunks; @NonNull private final String truncationText; + private final boolean truncateStart; - /** - * Create a {@link TruncatedString} with the provided chunks and truncation message. - * - * @param truncationText the message to be appended should maxSize be exceeded, e.g. - * "Some output is not shown here, see more on [Jenkins](url)." - * @param chunks a list of {@link CharSequence}s that are to be concatenated. - */ - private TruncatedString(@NonNull final String truncationText, @NonNull final List chunks) { + protected TruncatedString(@NonNull final String truncationText, final boolean reverse) { this.truncationText = Objects.requireNonNull(truncationText); - this.chunks = Objects.requireNonNull(chunks); + this.truncateStart = reverse; } /** @@ -41,12 +36,7 @@ private TruncatedString(@NonNull final String truncationText, @NonNull final Lis * @return a {@link TruncatedString} wrapping the provided input */ static TruncatedString fromString(final String string) { - return new TruncatedString.Builder().addText(string).build(); - } - - @Override - public String toString() { - return String.join("", chunks); + return new NewlineTruncatedString.Builder().withString(string).build(); } /** @@ -54,10 +44,10 @@ public String toString() { * * @return A string comprising the joined chunks. */ - @CheckForNull - public String build() { - return chunks.isEmpty() ? null : String.join("", chunks); - } + @Override + public abstract String toString(); + + protected abstract List getChunks(); /** * Builds the string such that it does not exceed maxSize, including the truncation string. @@ -68,41 +58,39 @@ public String build() { */ @CheckForNull public String build(final int maxSize) { - if (chunks.isEmpty()) { - return null; - } - String quickJoin = String.join("", chunks); - if (quickJoin.length() <= maxSize) { - return quickJoin; + List chunks = getChunks(); + if (truncateStart) { + Collections.reverse(chunks); } - StringBuilder builder = new StringBuilder(); - for (CharSequence chunk: chunks) { - if (builder.length() + chunk.length() + truncationText.length() < maxSize) { - builder.append(chunk); - } - else { - builder.append(truncationText); - break; - } - } - return builder.toString(); + return chunks.stream().collect(new Joiner(maxSize)); } + /** - * Builder for {@link TruncatedString}. + * Base builder for {@link TruncatedString}. + * + * @param the type of {@link TruncatedString} to build */ - public static class Builder { + public abstract static class Builder { private String truncationText = "Output truncated."; - private final List chunks = new ArrayList<>(); + private boolean truncateStart = false; + + protected String getTruncationText() { + return truncationText; + } + + protected boolean isTruncateStart() { + return truncateStart; + } + + protected abstract B self(); /** * Builds the {@link TruncatedString}. * * @return the build {@link TruncatedString}. */ - public TruncatedString build() { - return new TruncatedString(truncationText, chunks); - } + public abstract TruncatedString build(); /** * Sets the truncation text. @@ -111,22 +99,94 @@ public TruncatedString build() { * @return this builder */ @SuppressWarnings("HiddenField") - public Builder withTruncationText(@NonNull final String truncationText) { + public B withTruncationText(@NonNull final String truncationText) { this.truncationText = Objects.requireNonNull(truncationText); - return this; + return self(); } /** - * Adds a chunk of text to the buidler. + * Sets truncator to remove excess text from the start, rather than the end. * - * @param chunk the chunk of text to append to this builder - * @return this buidler + * @return this builder */ - public Builder addText(@NonNull final CharSequence chunk) { - this.chunks.add(Objects.requireNonNull(chunk)); - return this; + public B setTruncateStart() { + this.truncateStart = true; + return self(); } } + private class Joiner implements Collector { + + private final int maxLength; + + Joiner(final int maxLength) { + if (maxLength < truncationText.length()) { + throw new IllegalArgumentException("Maximum length is less than truncation text."); + } + this.maxLength = maxLength; + } + + @Override + public Supplier supplier() { + return Accumulator::new; + } + + @Override + public BiConsumer accumulator() { + return Accumulator::add; + } + + @Override + public BinaryOperator combiner() { + return Accumulator::combine; + } + + @Override + public Function finisher() { + return Accumulator::join; + } + + @Override + public Set characteristics() { + return Collections.emptySet(); + } + + private class Accumulator { + private final List chunks = new ArrayList<>(); + private int length = 0; + private boolean truncated = false; + + Accumulator combine(final Accumulator other) { + other.chunks.forEach(this::add); + return this; + } + + void add(final String chunk) { + if (truncated) { + return; + } + if (length + chunk.length() > maxLength) { + truncated = true; + return; + } + chunks.add(chunk); + length += chunk.length(); + } + + String join() { + if (truncateStart) { + Collections.reverse(chunks); + } + if (truncated) { + if (length + truncationText.length() > maxLength) { + chunks.remove(truncateStart ? 0 : chunks.size() - 1); + } + chunks.add(truncationText); + } + return String.join("", chunks); + } + } + } + } diff --git a/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java new file mode 100644 index 00000000..a15fe5b3 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java @@ -0,0 +1,79 @@ +package io.jenkins.plugins.checks.api; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class ChunkedTruncatedStringTest { + private static final String MESSAGE = "Truncated"; // length 9 + + @Test + public void shouldBuildStrings() { + ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() + .withTruncationText(MESSAGE); + builder.addText("Hello"); + assertThat(builder.build()).asString().isEqualTo("Hello"); + assertThat(builder.build().build(1000)).isEqualTo("Hello"); + builder.addText(", world!"); + assertThat(builder.build()).asString().isEqualTo("Hello, world!"); + assertThat(builder.build().build(1000)).isEqualTo("Hello, world!"); + } + + @Test + public void shouldTruncateStrings() { + ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() + .withTruncationText(MESSAGE); + builder.addText("xxxxxxxxxx"); // 10 + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxx"); + builder.addText("yyyyy"); // 5, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxxyyyyy"); + builder.addText("zzzzzzz"); // 7, does cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxxTruncated"); + } + + @Test + public void shouldHandleEdgeCases() { + ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() + .withTruncationText(MESSAGE); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.addText("xxxxxxxxxxxxxxx"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().build(5); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Maximum length is less than truncation text."); + } + + @Test + public void shouldHandleReversedChunking() { + ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() + .setTruncateStart() + .withTruncationText(MESSAGE); + builder.addText("zzzzz"); // 5 + assertThat(builder.build().build(20)).isEqualTo("zzzzz"); + builder.addText("xxxxx"); // 5, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("zzzzzxxxxx"); + builder.addText("ccccc"); // 5, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("zzzzzxxxxxccccc"); + builder.addText("aaaaaaa"); // 7, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("cccccaaaaaaaTruncated"); + } + + @Test + public void shouldHandleEdgeCasesReversed() { + ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() + .setTruncateStart() + .withTruncationText(MESSAGE); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.addText("xxxxxxxxxxxxxxx"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().build(5); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Maximum length is less than truncation text."); + } + +} \ No newline at end of file diff --git a/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java new file mode 100644 index 00000000..15061470 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java @@ -0,0 +1,78 @@ +package io.jenkins.plugins.checks.api; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class NewlineTruncatedStringTest { + private static final String MESSAGE = "Truncated"; // length 9 + + @Test + public void shouldBuildStrings() { + NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() + .withTruncationText(MESSAGE); + builder.withString("Hello"); + assertThat(builder.build()).asString().isEqualTo("Hello"); + assertThat(builder.build().build(1000)).isEqualTo("Hello"); + builder.withString("Hello,\n world!"); + assertThat(builder.build()).asString().isEqualTo("Hello,\n world!"); + assertThat(builder.build().build(1000)).isEqualTo("Hello,\n world!"); + } + + @Test + public void shouldTruncateStrings() { + NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() + .withTruncationText(MESSAGE); + builder.withString("xxxxxxxxx\n"); // 10 + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\n"); + builder.withString("xxxxxxxxx\nyyyy\n"); // 15, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nyyyy\n"); + builder.withString("xxxxxxxxx\nyyyy\nzzzzzz\n"); // 22, does cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nTruncated"); + } + + @Test + public void shouldHandleEdgeCases() { + NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() + .withTruncationText(MESSAGE); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.withString("xxxxxxxxxxxxxx\n"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().build(5); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Maximum length is less than truncation text."); + } + + @Test + public void shouldHandleReversedChunking() { + NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() + .setTruncateStart() + .withTruncationText(MESSAGE); + builder.withString("wwww\n"); // 5 + assertThat(builder.build().build(20)).isEqualTo("wwww\n"); + builder.withString("wwww\nxxxx\n"); // 10, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("wwww\nxxxx\n"); + builder.withString("wwww\nxxxx\nyyyy\n"); // 15, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("wwww\nxxxx\nyyyy\n"); + builder.withString("wwww\nxxxx\nyyyy\nzzzzzz\n"); // 22, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("yyyy\nzzzzzz\nTruncated"); + } + + @Test + public void shouldHandleEdgeCasesReversed() { + NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() + .setTruncateStart() + .withTruncationText(MESSAGE); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.withString("xxxxxxxxxxxxxx\n"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().build(5); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Maximum length is less than truncation text."); + } +} \ No newline at end of file From 5fa067d9aa776d15bd0256a4fb1f5e4bd1cd5c1d Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 9 Jan 2021 20:33:32 +0000 Subject: [PATCH 3/5] Consolidate truncated strings into single class --- .../checks/api/ChunkedTruncatedString.java | 63 --------- .../checks/api/NewlineTruncatedString.java | 63 --------- .../plugins/checks/api/TruncatedString.java | 82 ++++++++---- .../plugins/checks/ArchitectureTest.java | 6 +- .../api/ChunkedTruncatedStringTest.java | 79 ------------ .../api/NewlineTruncatedStringTest.java | 78 ----------- .../checks/api/TruncatedStringTest.java | 122 ++++++++++++++++++ 7 files changed, 179 insertions(+), 314 deletions(-) delete mode 100644 src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java delete mode 100644 src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java delete mode 100644 src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java delete mode 100644 src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java create mode 100644 src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java diff --git a/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java deleted file mode 100644 index 748c0304..00000000 --- a/src/main/java/io/jenkins/plugins/checks/api/ChunkedTruncatedString.java +++ /dev/null @@ -1,63 +0,0 @@ -package io.jenkins.plugins.checks.api; - -import edu.umd.cs.findbugs.annotations.NonNull; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Objects; - -/** - * Implementation of {@link TruncatedString} that truncates strings to the nearest supplied chunk. This allows for - * ensuring that, for example, markdown syntax is not truncated midway through a ``` block, or a table, that would - * result in poor formatting or confusing output. - */ -@SuppressWarnings("PMD.MissingStaticMethodInNonInstantiatableClass") -public class ChunkedTruncatedString extends TruncatedString { - - @NonNull - private final List chunks; - - private ChunkedTruncatedString(@NonNull final String truncationText, final boolean truncateStart, @NonNull final List chunks) { - super(truncationText, truncateStart); - this.chunks = Collections.unmodifiableList(chunks); - } - - @Override - public String toString() { - return String.join("", chunks); - } - - @Override - protected List getChunks() { - return new ArrayList<>(chunks); - } - - /** - * Builder for {@link ChunkedTruncatedString}. - */ - public static class Builder extends TruncatedString.Builder { - private final List chunks = new ArrayList<>(); - - @Override - protected Builder self() { - return this; - } - - @Override - public TruncatedString build() { - return new ChunkedTruncatedString(getTruncationText(), isTruncateStart(), chunks); - } - - /** - * Adds a chunk of text to the buidler. - * - * @param chunk the chunk of text to append to this builder - * @return this buidler - */ - public Builder addText(@NonNull final String chunk) { - this.chunks.add(Objects.requireNonNull(chunk)); - return this; - } - } -} diff --git a/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java deleted file mode 100644 index cada6479..00000000 --- a/src/main/java/io/jenkins/plugins/checks/api/NewlineTruncatedString.java +++ /dev/null @@ -1,63 +0,0 @@ -package io.jenkins.plugins.checks.api; - -import edu.umd.cs.findbugs.annotations.NonNull; - -import java.util.Arrays; -import java.util.List; - -/** - * Implementation of {@link TruncatedString} that truncates strings to the nearest newline. - */ -@SuppressWarnings("PMD.MissingStaticMethodInNonInstantiatableClass") -public class NewlineTruncatedString extends TruncatedString { - - @NonNull - private final String string; - - private NewlineTruncatedString(@NonNull final String truncationText, final boolean truncateStart, @NonNull final String string) { - super(truncationText, truncateStart); - this.string = string; - } - - @Override - public String toString() { - return string; - } - - @Override - protected List getChunks() { - return Arrays.asList(string.split("(?<=\r?\n)")); - } - - /** - * Builder for {@link NewlineTruncatedString}. - */ - public static class Builder extends TruncatedString.Builder { - - private String string = ""; - - @Override - protected Builder self() { - return this; - } - - @Override - public TruncatedString build() { - return new NewlineTruncatedString(getTruncationText(), isTruncateStart(), string); - } - - /** - * Set the string to truncate. - * - * @param string the string to truncate - * @return this builder - */ - @SuppressWarnings("HiddenField") - public Builder withString(final String string) { - this.string = string; - return this; - } - - } - -} diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java index 4d6aebc7..7eb2ea81 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java +++ b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java @@ -18,15 +18,21 @@ * chunks of complete markdown until an overflow is detected, at which point a message will be added and all future * additions will be silently discarded. */ -public abstract class TruncatedString { +public class TruncatedString { + @NonNull + private final List chunks; @NonNull private final String truncationText; private final boolean truncateStart; + private final boolean chunkOnNewlines; + - protected TruncatedString(@NonNull final String truncationText, final boolean reverse) { + private TruncatedString(@NonNull final List chunks, @NonNull final String truncationText, final boolean truncateStart, final boolean chunkOnNewlines) { + this.chunks = Collections.unmodifiableList(Objects.requireNonNull(chunks)); this.truncationText = Objects.requireNonNull(truncationText); - this.truncateStart = reverse; + this.truncateStart = truncateStart; + this.chunkOnNewlines = chunkOnNewlines; } /** @@ -36,7 +42,7 @@ protected TruncatedString(@NonNull final String truncationText, final boolean re * @return a {@link TruncatedString} wrapping the provided input */ static TruncatedString fromString(final String string) { - return new NewlineTruncatedString.Builder().withString(string).build(); + return new Builder().setChunkOnNewlines().addText(string).build(); } /** @@ -45,9 +51,16 @@ static TruncatedString fromString(final String string) { * @return A string comprising the joined chunks. */ @Override - public abstract String toString(); + public String toString() { + return String.join("", chunks); + } - protected abstract List getChunks(); + private List getChunks() { + if (chunkOnNewlines) { + return Arrays.asList(String.join("", chunks).split("(?<=\r?\n)")); + } + return new ArrayList<>(chunks); + } /** * Builds the string such that it does not exceed maxSize, including the truncation string. @@ -58,39 +71,42 @@ static TruncatedString fromString(final String string) { */ @CheckForNull public String build(final int maxSize) { - List chunks = getChunks(); + List parts = getChunks(); if (truncateStart) { - Collections.reverse(chunks); + Collections.reverse(parts); } - return chunks.stream().collect(new Joiner(maxSize)); + return parts.stream().collect(new Joiner(maxSize)); } /** - * Base builder for {@link TruncatedString}. - * - * @param the type of {@link TruncatedString} to build + * Builder for {@link TruncatedString}. */ - public abstract static class Builder { + public static class Builder { private String truncationText = "Output truncated."; private boolean truncateStart = false; - - protected String getTruncationText() { - return truncationText; - } - - protected boolean isTruncateStart() { - return truncateStart; - } - - protected abstract B self(); + private boolean chunkOnNewlines = false; + private final List chunks = new ArrayList<>(); /** * Builds the {@link TruncatedString}. * * @return the build {@link TruncatedString}. */ - public abstract TruncatedString build(); + public TruncatedString build() { + return new TruncatedString(chunks, truncationText, truncateStart, chunkOnNewlines); + } + + /** + * Adds a chunk of text to the buidler. + * + * @param text the chunk of text to append to this builder + * @return this builder + */ + public Builder addText(@NonNull final String text) { + this.chunks.add(Objects.requireNonNull(text)); + return this; + } /** * Sets the truncation text. @@ -99,9 +115,9 @@ protected boolean isTruncateStart() { * @return this builder */ @SuppressWarnings("HiddenField") - public B withTruncationText(@NonNull final String truncationText) { + public Builder withTruncationText(@NonNull final String truncationText) { this.truncationText = Objects.requireNonNull(truncationText); - return self(); + return this; } /** @@ -109,9 +125,19 @@ public B withTruncationText(@NonNull final String truncationText) { * * @return this builder */ - public B setTruncateStart() { + public Builder setTruncateStart() { this.truncateStart = true; - return self(); + return this; + } + + /** + * Sets truncator to chunk on newlines rather than the chunks. + * + * @return this builder + */ + public Builder setChunkOnNewlines() { + this.chunkOnNewlines = true; + return this; } } diff --git a/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java b/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java index 6c292969..1581cfa6 100644 --- a/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java +++ b/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java @@ -16,9 +16,9 @@ class ArchitectureTest { @ArchTest static final ArchRule NO_JENKINS_INSTANCE_CALL = PluginArchitectureRules.NO_JENKINS_INSTANCE_CALL; - - @ArchTest - static final ArchRule NO_PUBLIC_TEST_CLASSES = PluginArchitectureRules.NO_PUBLIC_TEST_CLASSES; +// Todo: this seems to prevent paramteized non-integration tests. +// @ArchTest +// static final ArchRule NO_PUBLIC_TEST_CLASSES = PluginArchitectureRules.NO_PUBLIC_TEST_CLASSES; @ArchTest static final ArchRule NO_TEST_API_CALLED = ArchitectureRules.NO_TEST_API_CALLED; diff --git a/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java deleted file mode 100644 index a15fe5b3..00000000 --- a/src/test/java/io/jenkins/plugins/checks/api/ChunkedTruncatedStringTest.java +++ /dev/null @@ -1,79 +0,0 @@ -package io.jenkins.plugins.checks.api; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -class ChunkedTruncatedStringTest { - private static final String MESSAGE = "Truncated"; // length 9 - - @Test - public void shouldBuildStrings() { - ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() - .withTruncationText(MESSAGE); - builder.addText("Hello"); - assertThat(builder.build()).asString().isEqualTo("Hello"); - assertThat(builder.build().build(1000)).isEqualTo("Hello"); - builder.addText(", world!"); - assertThat(builder.build()).asString().isEqualTo("Hello, world!"); - assertThat(builder.build().build(1000)).isEqualTo("Hello, world!"); - } - - @Test - public void shouldTruncateStrings() { - ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() - .withTruncationText(MESSAGE); - builder.addText("xxxxxxxxxx"); // 10 - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxx"); - builder.addText("yyyyy"); // 5, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxxyyyyy"); - builder.addText("zzzzzzz"); // 7, does cause overflow - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxxxTruncated"); - } - - @Test - public void shouldHandleEdgeCases() { - ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() - .withTruncationText(MESSAGE); - assertThat(builder.build().build(10)).isEqualTo(""); - assertThat(builder.build()).asString().isEqualTo(""); - builder.addText("xxxxxxxxxxxxxxx"); // 15 - assertThat(builder.build().build(10)).isEqualTo("Truncated"); - assertThatThrownBy(() -> { - builder.build().build(5); - }).isInstanceOf(IllegalArgumentException.class) - .hasMessage("Maximum length is less than truncation text."); - } - - @Test - public void shouldHandleReversedChunking() { - ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() - .setTruncateStart() - .withTruncationText(MESSAGE); - builder.addText("zzzzz"); // 5 - assertThat(builder.build().build(20)).isEqualTo("zzzzz"); - builder.addText("xxxxx"); // 5, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("zzzzzxxxxx"); - builder.addText("ccccc"); // 5, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("zzzzzxxxxxccccc"); - builder.addText("aaaaaaa"); // 7, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("cccccaaaaaaaTruncated"); - } - - @Test - public void shouldHandleEdgeCasesReversed() { - ChunkedTruncatedString.Builder builder = new ChunkedTruncatedString.Builder() - .setTruncateStart() - .withTruncationText(MESSAGE); - assertThat(builder.build().build(10)).isEqualTo(""); - assertThat(builder.build()).asString().isEqualTo(""); - builder.addText("xxxxxxxxxxxxxxx"); // 15 - assertThat(builder.build().build(10)).isEqualTo("Truncated"); - assertThatThrownBy(() -> { - builder.build().build(5); - }).isInstanceOf(IllegalArgumentException.class) - .hasMessage("Maximum length is less than truncation text."); - } - -} \ No newline at end of file diff --git a/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java deleted file mode 100644 index 15061470..00000000 --- a/src/test/java/io/jenkins/plugins/checks/api/NewlineTruncatedStringTest.java +++ /dev/null @@ -1,78 +0,0 @@ -package io.jenkins.plugins.checks.api; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -class NewlineTruncatedStringTest { - private static final String MESSAGE = "Truncated"; // length 9 - - @Test - public void shouldBuildStrings() { - NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() - .withTruncationText(MESSAGE); - builder.withString("Hello"); - assertThat(builder.build()).asString().isEqualTo("Hello"); - assertThat(builder.build().build(1000)).isEqualTo("Hello"); - builder.withString("Hello,\n world!"); - assertThat(builder.build()).asString().isEqualTo("Hello,\n world!"); - assertThat(builder.build().build(1000)).isEqualTo("Hello,\n world!"); - } - - @Test - public void shouldTruncateStrings() { - NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() - .withTruncationText(MESSAGE); - builder.withString("xxxxxxxxx\n"); // 10 - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\n"); - builder.withString("xxxxxxxxx\nyyyy\n"); // 15, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nyyyy\n"); - builder.withString("xxxxxxxxx\nyyyy\nzzzzzz\n"); // 22, does cause overflow - assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nTruncated"); - } - - @Test - public void shouldHandleEdgeCases() { - NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() - .withTruncationText(MESSAGE); - assertThat(builder.build().build(10)).isEqualTo(""); - assertThat(builder.build()).asString().isEqualTo(""); - builder.withString("xxxxxxxxxxxxxx\n"); // 15 - assertThat(builder.build().build(10)).isEqualTo("Truncated"); - assertThatThrownBy(() -> { - builder.build().build(5); - }).isInstanceOf(IllegalArgumentException.class) - .hasMessage("Maximum length is less than truncation text."); - } - - @Test - public void shouldHandleReversedChunking() { - NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() - .setTruncateStart() - .withTruncationText(MESSAGE); - builder.withString("wwww\n"); // 5 - assertThat(builder.build().build(20)).isEqualTo("wwww\n"); - builder.withString("wwww\nxxxx\n"); // 10, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("wwww\nxxxx\n"); - builder.withString("wwww\nxxxx\nyyyy\n"); // 15, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("wwww\nxxxx\nyyyy\n"); - builder.withString("wwww\nxxxx\nyyyy\nzzzzzz\n"); // 22, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("yyyy\nzzzzzz\nTruncated"); - } - - @Test - public void shouldHandleEdgeCasesReversed() { - NewlineTruncatedString.Builder builder = new NewlineTruncatedString.Builder() - .setTruncateStart() - .withTruncationText(MESSAGE); - assertThat(builder.build().build(10)).isEqualTo(""); - assertThat(builder.build()).asString().isEqualTo(""); - builder.withString("xxxxxxxxxxxxxx\n"); // 15 - assertThat(builder.build().build(10)).isEqualTo("Truncated"); - assertThatThrownBy(() -> { - builder.build().build(5); - }).isInstanceOf(IllegalArgumentException.class) - .hasMessage("Maximum length is less than truncation text."); - } -} \ No newline at end of file diff --git a/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java new file mode 100644 index 00000000..b4e00736 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java @@ -0,0 +1,122 @@ +package io.jenkins.plugins.checks.api; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Test behavior of the {@link TruncatedString}. + */ +@SuppressWarnings({"VisibilityModifier", "MissingJavadocMethod"}) +@RunWith(Parameterized.class) +public class TruncatedStringTest { + private static final String MESSAGE = "Truncated"; // length 9 + + /** + * Human readable test name. + */ + @Parameterized.Parameter + public String testName; + + /** + * Parameter for chunking on new lines (or not!). + */ + @Parameterized.Parameter(1) + public boolean chunkOnNewlines; + + @Parameterized.Parameters(name = "{0}") + public static Object[][] parameters() { + return new Object[][]{ + {"Chunks", false}, + {"Newlines", true} + }; + } + + private TruncatedString.Builder getBuilder() { + TruncatedString.Builder builder = new TruncatedString.Builder() + .withTruncationText(MESSAGE); + if (chunkOnNewlines) { + return builder.setChunkOnNewlines(); + } + return builder; + } + + @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"); + builder.addText(", world!"); + assertThat(builder.build()).asString().isEqualTo("Hello\n, world!"); + assertThat(builder.build().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"); + builder.addText("yyyy\n"); // 5, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nyyyy\n"); + builder.addText("zzzzzz\n"); // 7, does cause overflow + assertThat(builder.build().build(20)).isEqualTo("xxxxxxxxx\nTruncated"); + } + + @Test + public void shouldHandleEdgeCases() { + TruncatedString.Builder builder = getBuilder(); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.addText("xxxxxxxxxxxxxx\n"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().build(5); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Maximum length is less than truncation text."); + } + + @Test + public void shouldHandleReversedChunking() { + TruncatedString.Builder builder = getBuilder() + .setTruncateStart(); + builder.addText("zzzz\n"); // 5 + assertThat(builder.build().build(20)).isEqualTo("zzzz\n"); + builder.addText("xxxx\n"); // 5, doesn't cause overflow + assertThat(builder.build().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"); + builder.addText("aaaaaa\n"); // 7, doesn't cause overflow + assertThat(builder.build().build(20)).isEqualTo("cccc\naaaaaa\nTruncated"); + } + + @Test + public void shouldHandleEdgeCasesReversed() { + TruncatedString.Builder builder = getBuilder() + .setTruncateStart(); + assertThat(builder.build().build(10)).isEqualTo(""); + assertThat(builder.build()).asString().isEqualTo(""); + builder.addText("xxxxxxxxxxxxxx\n"); // 15 + assertThat(builder.build().build(10)).isEqualTo("Truncated"); + assertThatThrownBy(() -> { + builder.build().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"); + + builder = getBuilder(); + builder.addText("wwww\n"); // 5 + builder.addText("xxxx\nyyyy\nzzzzz\n"); // 16 + assertThat(builder.build().build(20)).isEqualTo(chunkOnNewlines ? "wwww\nxxxx\nTruncated" : "wwww\nTruncated"); + } +} From a3201672282a91a73046fac9cd3552c12222bec5 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sun, 10 Jan 2021 11:40:09 +0000 Subject: [PATCH 4/5] Review feedback --- .../java/io/jenkins/plugins/checks/api/ChecksOutput.java | 4 ++-- .../io/jenkins/plugins/checks/api/TruncatedString.java | 4 +--- .../java/io/jenkins/plugins/checks/ArchitectureTest.java | 9 ++++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java index 9f075e3e..4d11209a 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java +++ b/src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java @@ -55,7 +55,7 @@ public Optional getSummary() { * @return Summary, truncated to maxSize with truncation message if appropriate. */ public Optional getSummary(final int maxSize) { - return Optional.ofNullable(summary).flatMap(s -> Optional.ofNullable(s.build(maxSize))); + return Optional.ofNullable(summary).map(s -> s.build(maxSize)); } public Optional getText() { @@ -69,7 +69,7 @@ public Optional getText() { * @return Text, truncated to maxSize with truncation message if appropriate. */ public Optional getText(final int maxSize) { - return Optional.ofNullable(text).flatMap(s -> Optional.ofNullable(s.build(maxSize))); + return Optional.ofNullable(text).map(s -> s.build(maxSize)); } public List getChecksAnnotations() { diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java index 7eb2ea81..bbadb640 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java +++ b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java @@ -1,6 +1,5 @@ package io.jenkins.plugins.checks.api; -import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.*; @@ -69,7 +68,6 @@ private List getChunks() { * @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. */ - @CheckForNull public String build(final int maxSize) { List parts = getChunks(); if (truncateStart) { @@ -98,7 +96,7 @@ public TruncatedString build() { } /** - * Adds a chunk of text to the buidler. + * Adds a chunk of text to the builder. * * @param text the chunk of text to append to this builder * @return this builder diff --git a/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java b/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java index 1581cfa6..e05482eb 100644 --- a/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java +++ b/src/test/java/io/jenkins/plugins/checks/ArchitectureTest.java @@ -3,8 +3,10 @@ import com.tngtech.archunit.junit.AnalyzeClasses; import com.tngtech.archunit.junit.ArchTest; import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.elements.ClassesShouldConjunction; import edu.hm.hafner.util.ArchitectureRules; import io.jenkins.plugins.util.PluginArchitectureRules; +import org.junit.runner.RunWith; /** * Defines several architecture rules for the static analysis model and parsers. @@ -16,9 +18,10 @@ class ArchitectureTest { @ArchTest static final ArchRule NO_JENKINS_INSTANCE_CALL = PluginArchitectureRules.NO_JENKINS_INSTANCE_CALL; -// Todo: this seems to prevent paramteized non-integration tests. -// @ArchTest -// static final ArchRule NO_PUBLIC_TEST_CLASSES = PluginArchitectureRules.NO_PUBLIC_TEST_CLASSES; + + @ArchTest + static final ArchRule NO_PUBLIC_TEST_CLASSES = ((ClassesShouldConjunction) PluginArchitectureRules.NO_PUBLIC_TEST_CLASSES) + .andShould().notBeAnnotatedWith(RunWith.class); // Allow for JUnit4 tests. @ArchTest static final ArchRule NO_TEST_API_CALLED = ArchitectureRules.NO_TEST_API_CALLED; From e92aa81c9bc822b5ae65bddafedbd8d2ab6cafab Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sun, 10 Jan 2021 11:46:16 +0000 Subject: [PATCH 5/5] Move trunction text to start when truncating from start, decouple Joiner from TruncatedString --- .../plugins/checks/api/TruncatedString.java | 25 +++++++++++-------- .../checks/api/TruncatedStringTest.java | 4 +-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java index bbadb640..d78be4eb 100644 --- a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java +++ b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java @@ -73,7 +73,11 @@ public String build(final int maxSize) { if (truncateStart) { Collections.reverse(parts); } - return parts.stream().collect(new Joiner(maxSize)); + List truncatedParts = parts.stream().collect(new Joiner(truncationText, maxSize)); + if (truncateStart) { + Collections.reverse(truncatedParts); + } + return String.join("", truncatedParts); } @@ -140,14 +144,16 @@ public Builder setChunkOnNewlines() { } - private class Joiner implements Collector { + private static class Joiner implements Collector> { private final int maxLength; + private final String truncationText; - Joiner(final int maxLength) { + Joiner(final String truncationText, final int maxLength) { if (maxLength < truncationText.length()) { throw new IllegalArgumentException("Maximum length is less than truncation text."); } + this.truncationText = truncationText; this.maxLength = maxLength; } @@ -167,8 +173,8 @@ public BinaryOperator combiner() { } @Override - public Function finisher() { - return Accumulator::join; + public Function> finisher() { + return Accumulator::truncate; } @Override @@ -198,17 +204,14 @@ void add(final String chunk) { length += chunk.length(); } - String join() { - if (truncateStart) { - Collections.reverse(chunks); - } + List truncate() { if (truncated) { if (length + truncationText.length() > maxLength) { - chunks.remove(truncateStart ? 0 : chunks.size() - 1); + chunks.remove(chunks.size() - 1); } chunks.add(truncationText); } - return String.join("", chunks); + return chunks; } } } diff --git a/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java index b4e00736..a1ef9f0a 100644 --- a/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java +++ b/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java @@ -89,8 +89,8 @@ public void shouldHandleReversedChunking() { assertThat(builder.build().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"); - builder.addText("aaaaaa\n"); // 7, doesn't cause overflow - assertThat(builder.build().build(20)).isEqualTo("cccc\naaaaaa\nTruncated"); + builder.addText("aaaaaa\n"); // 7, does cause overflow + assertThat(builder.build().build(20)).isEqualTo("Truncatedcccc\naaaaaa\n"); } @Test