From 531bfd0e0f8b279ac819bf71629f7a90724f493f Mon Sep 17 00:00:00 2001 From: Sathish Kumar Subramani Date: Mon, 2 Jan 2023 18:41:53 +0100 Subject: [PATCH 1/2] Omit start_column, end_column for multi-line annotation When start_line and end_line have different values & start_column or end_column has value, then Github API throws the following error while creating or updating a Check-run `HTTP 422: {"resource":"CheckRun","code":"invalid","field":"annotations"}` --- .../com/spotify/github/v3/checks/Annotation.java | 13 ++++++++++++- .../spotify/github/v3/checks/AnnotationTest.java | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/spotify/github/v3/checks/Annotation.java b/src/main/java/com/spotify/github/v3/checks/Annotation.java index 4bb953b4..1c18cf2c 100644 --- a/src/main/java/com/spotify/github/v3/checks/Annotation.java +++ b/src/main/java/com/spotify/github/v3/checks/Annotation.java @@ -125,7 +125,7 @@ public interface Annotation { */ @Value.Check @SuppressWarnings("checkstyle:magicnumber") - default void check() { + default Annotation check() { // max values from https://docs.github.com/en/rest/checks/runs Preconditions.checkState(title().map(String::length).orElse(0) <= 255, "'title' exceeded max length of 255"); @@ -133,5 +133,16 @@ default void check() { "'message' exceeded max length of 64kB"); Preconditions.checkState(rawDetails().map(String::length).orElse(0) <= 64 * 1024, "'rawDetails' exceeded max length of 64kB"); + + // Omit this (start_column, end_column) parameter if start_line and end_line have different values + // from https://docs.github.com/en/rest/checks/runs + if (startLine() != endLine()) { + return ImmutableAnnotation.builder() + .from(this) + .startColumn(Optional.empty()) + .endColumn(Optional.empty()) + .build(); + } + return this; } } diff --git a/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java b/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java index 2194259b..f5ffee4b 100644 --- a/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java +++ b/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java @@ -37,6 +37,8 @@ private Builder builder() { .path("path") .startLine(1) .endLine(2) + .startColumn(1) + .endColumn(9) .annotationLevel(AnnotationLevel.notice); } @@ -79,4 +81,16 @@ public void serializesWithEmptyFields() { String expected = "{\"path\":\"\",\"annotation_level\":\"notice\",\"message\":\"\",\"title\":\"\",\"start_line\":1,\"end_line\":2}"; assertThat(serializedAnnotation, is(expected)); } + + @Test + public void clearsColumnFieldsForMultiLineAnnotation() { + Annotation annotationSpanningMultipleLines = builder().startLine(1).endLine(2).build(); + String serializedAnnotation = Json.create().toJsonUnchecked(annotationSpanningMultipleLines); + String serializedWithoutColumns = "{\"path\":\"path\",\"annotation_level\":\"notice\",\"message\":\"message\",\"title\":\"title\",\"raw_details\":\"rawDetails\",\"start_line\":1,\"end_line\":2"; + assertThat(serializedAnnotation, is(serializedWithoutColumns + "}")); + + Annotation annotationOnSingleLine = builder().startLine(1).endLine(1).build(); + serializedAnnotation = Json.create().toJsonUnchecked(annotationOnSingleLine); + assertThat(serializedAnnotation, is(serializedWithoutColumns + ",\"start_column\":1,\"end_column\":9}")); + } } From 1715cda5a35c201e9355f94621d61d7ddf08f73d Mon Sep 17 00:00:00 2001 From: Sathish Kumar Subramani Date: Tue, 3 Jan 2023 09:02:10 +0100 Subject: [PATCH 2/2] Fix failing tests --- .../spotify/github/v3/checks/Annotation.java | 2 +- .../github/v3/checks/AnnotationTest.java | 24 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/spotify/github/v3/checks/Annotation.java b/src/main/java/com/spotify/github/v3/checks/Annotation.java index 1c18cf2c..7e62f9e1 100644 --- a/src/main/java/com/spotify/github/v3/checks/Annotation.java +++ b/src/main/java/com/spotify/github/v3/checks/Annotation.java @@ -136,7 +136,7 @@ default Annotation check() { // Omit this (start_column, end_column) parameter if start_line and end_line have different values // from https://docs.github.com/en/rest/checks/runs - if (startLine() != endLine()) { + if (startLine() != endLine() && (startColumn().isPresent() || endColumn().isPresent())) { return ImmutableAnnotation.builder() .from(this) .startColumn(Optional.empty()) diff --git a/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java b/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java index f5ffee4b..934b0426 100644 --- a/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java +++ b/src/test/java/com/spotify/github/v3/checks/AnnotationTest.java @@ -22,10 +22,13 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import com.spotify.github.jackson.Json; import com.spotify.github.v3.checks.ImmutableAnnotation.Builder; +import java.util.Optional; import org.junit.Test; public class AnnotationTest { @@ -84,13 +87,20 @@ public void serializesWithEmptyFields() { @Test public void clearsColumnFieldsForMultiLineAnnotation() { - Annotation annotationSpanningMultipleLines = builder().startLine(1).endLine(2).build(); - String serializedAnnotation = Json.create().toJsonUnchecked(annotationSpanningMultipleLines); - String serializedWithoutColumns = "{\"path\":\"path\",\"annotation_level\":\"notice\",\"message\":\"message\",\"title\":\"title\",\"raw_details\":\"rawDetails\",\"start_line\":1,\"end_line\":2"; - assertThat(serializedAnnotation, is(serializedWithoutColumns + "}")); + Annotation multiLineAnnotation = builder().startLine(1).endLine(2).build(); + assertTrue(multiLineAnnotation.startColumn().isEmpty()); + assertTrue(multiLineAnnotation.endColumn().isEmpty()); - Annotation annotationOnSingleLine = builder().startLine(1).endLine(1).build(); - serializedAnnotation = Json.create().toJsonUnchecked(annotationOnSingleLine); - assertThat(serializedAnnotation, is(serializedWithoutColumns + ",\"start_column\":1,\"end_column\":9}")); + Annotation anotherMultiLineAnnotation = builder().startLine(1).endLine(2).startColumn(Optional.empty()).endColumn(1).build(); + assertTrue(anotherMultiLineAnnotation.startColumn().isEmpty()); + assertTrue(anotherMultiLineAnnotation.endColumn().isEmpty()); + + Annotation yetAnotherMultiLineAnnotation = builder().startLine(1).endLine(2).startColumn(1).endColumn(Optional.empty()).build(); + assertTrue(yetAnotherMultiLineAnnotation.startColumn().isEmpty()); + assertTrue(yetAnotherMultiLineAnnotation.endColumn().isEmpty()); + + Annotation singleLineAnnotation = builder().startLine(1).endLine(1).build(); + assertEquals(1, singleLineAnnotation.startColumn().orElse(0)); + assertEquals(9, singleLineAnnotation.endColumn().orElse(0)); } }