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

Allow for moving truncation into the implementing API #68

Merged
merged 6 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

<properties>
<java.level>8</java.level>
<revision>1.2.1</revision>
<revision>1.3.0</revision>
<changelist>-SNAPSHOT</changelist>

<!-- Jenkins Plug-in Dependencies Versions -->
Expand Down
74 changes: 65 additions & 9 deletions src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
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,
* annotations, etc.
*/
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<ChecksAnnotation> annotations;
private final List<ChecksImage> 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<ChecksAnnotation> annotations, final List<ChecksImage> images) {
this.title = title;
this.summary = summary;
Expand All @@ -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());
}

Expand All @@ -43,11 +47,31 @@ public Optional<String> getTitle() {
}

public Optional<String> 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<String> getSummary(final int maxSize) {
return Optional.ofNullable(summary).flatMap(s -> Optional.ofNullable(s.build(maxSize)));
XiongKezhi marked this conversation as resolved.
Show resolved Hide resolved
}

public Optional<String> 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<String> getText(final int maxSize) {
return Optional.ofNullable(text).flatMap(s -> Optional.ofNullable(s.build(maxSize)));
XiongKezhi marked this conversation as resolved.
Show resolved Hide resolved
}

public List<ChecksAnnotation> getChecksAnnotations() {
Expand All @@ -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<ChecksAnnotation> annotations;
private List<ChecksImage> images;

Expand Down Expand Up @@ -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}.
*
* <p>
* Note that for the GitHub check runs, the {@code summary} supports Markdown.
* <p>
*
* @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;
}
Expand All @@ -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.
*
* <p>
* Note that for a GitHub check run, the {@code text} supports Markdown.
* <p>
*
* @param text
* the details description in Markdown as a {@link TruncatedString}
* @return this builder
*/
@SuppressWarnings("HiddenField")
public ChecksOutputBuilder withText(final TruncatedString text) {
Copy link
Contributor

Choose a reason for hiding this comment

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

adding the truncated string builder in line 66 make sense since it makes things convenient for implementations, but since we already have details URL, I'm not sure the see more link would be necessary for consumers, if not, the truncated string build is unnecessary as well IMO.

Copy link
Member

Choose a reason for hiding this comment

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

#68 (comment)

the see more link was just a helpful extra added in junit it could probably be skipped but truncating is needed afaict

this.text = requireNonNull(text);
return this;
}
Expand Down
132 changes: 132 additions & 0 deletions src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java
Original file line number Diff line number Diff line change
@@ -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<CharSequence> 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<CharSequence> 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);
Copy link
Member

Choose a reason for hiding this comment

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

what do you think of the statement:

the end of the build log is more interesting than the beginning, there should be more fine grained control over the truncation?

This is more relevant to #66

or should we just say YAGNI and improve it later if required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable, and certainly easy enough to implement.

I also think there’s cause to say “allow newine truncation”, so that if you know you’re just putting plain text in (e.g. a large build log) as a single chunk, you can do a new line truncation. Combine that with reverse truncation and you have everything I think.

}

/**
* 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<CharSequence> 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;
}

}

}