Skip to content

Commit

Permalink
Refactored ChecksAction and ChecksImage to immutable and use construc…
Browse files Browse the repository at this point in the history
…tor with all @nullable parameters
  • Loading branch information
XiongKezhi committed Jun 30, 2020
1 parent fcc5b34 commit 58d74e2
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 125 deletions.
35 changes: 14 additions & 21 deletions src/main/java/io/jenkins/plugins/checks/api/ChecksAction.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.jenkins.plugins.checks.api;

import static java.util.Objects.*;
import java.util.Optional;

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

public class ChecksAction {
private final String label;
Expand All @@ -22,31 +24,22 @@ public class ChecksAction {
* @param identifier
* a reference for the action on the integrator's system
*/
public ChecksAction(final String label, final String description, final String identifier) {
this.label = requireNonNull(label);
this.description = requireNonNull(description);
this.identifier = requireNonNull(identifier);
}

/**
* Copy constructor of the {@link ChecksOutput}.
*
* @param that
* the source to copy from
*/
public ChecksAction(final ChecksAction that) {
this(that.getLabel(), that.getDescription(), that.getIdentifier());
public ChecksAction(@Nullable final String label, @Nullable final String description,
@Nullable final String identifier) {
this.label = label;
this.description = description;
this.identifier = identifier;
}

public String getLabel() {
return label;
public Optional<String> getLabel() {
return Optional.ofNullable(label);
}

public String getDescription() {
return description;
public Optional<String> getDescription() {
return Optional.ofNullable(description);
}

public String getIdentifier() {
return identifier;
public Optional<String> getIdentifier() {
return Optional.ofNullable(identifier);
}
}
14 changes: 3 additions & 11 deletions src/main/java/io/jenkins/plugins/checks/api/ChecksDetails.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -122,7 +121,7 @@ public static class ChecksDetailsBuilder {
*/
public ChecksDetailsBuilder() {
this.conclusion = ChecksConclusion.NONE;
this.actions = Collections.emptyList();
this.actions = new ArrayList<>();
}

/**
Expand Down Expand Up @@ -248,10 +247,7 @@ public ChecksDetailsBuilder withOutput(final ChecksOutput output) {
* @throws NullPointerException if the {@code actions} is null
*/
public ChecksDetailsBuilder withActions(final List<ChecksAction> actions) {
this.actions = requireNonNull(actions).stream()
.map(ChecksAction::new)
.collect(Collectors.toList()
);
this.actions = new ArrayList<>(requireNonNull(actions));
return this;
}

Expand All @@ -263,11 +259,7 @@ public ChecksDetailsBuilder withActions(final List<ChecksAction> actions) {
* @return this builder
*/
public ChecksDetailsBuilder addAction(final ChecksAction action) {
requireNonNull(action);
if (actions == Collections.EMPTY_LIST) {
actions = new ArrayList<>();
}
actions.add(new ChecksAction(action));
actions.add(requireNonNull(action));
return this;
}

Expand Down
46 changes: 13 additions & 33 deletions src/main/java/io/jenkins/plugins/checks/api/ChecksImage.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,56 +2,48 @@

import java.util.Optional;

import static java.util.Objects.*;
import edu.umd.cs.findbugs.annotations.Nullable;

/**
* An image of a check. Users may use a image to show the code coverage, issues trend, etc.
*/
public class ChecksImage {
private final String alt;
private final String imageUrl;
private String caption;
private final String caption;

/**
* Construct an image with required parameters.
* Constructs an image with all parameters.
*
* @param alt
* the alternative text for the image
* @param imageUrl
* the full URL of the image
* @param caption
* a short description of the image
*/
public ChecksImage(final String alt, final String imageUrl) {
this.alt = requireNonNull(alt);
this.imageUrl = requireNonNull(imageUrl);
}

/**
* Copy constructor.
*
* @param that
* the source
*/
public ChecksImage(final ChecksImage that) {
this(that.getAlt(), that.getImageUrl());
this.caption = that.getCaption().orElse(null);
public ChecksImage(@Nullable final String alt, @Nullable final String imageUrl, @Nullable final String caption) {
this.alt = alt;
this.imageUrl = imageUrl;
this.caption = caption;
}

/**
* Returns the alternative text for the image.
*
* @return the alternative text for the image
*/
public String getAlt() {
return alt;
public Optional<String> getAlt() {
return Optional.ofNullable(alt);
}

/**
* Returns the image URL.
*
* @return the image URL
*/
public String getImageUrl() {
return imageUrl;
public Optional<String> getImageUrl() {
return Optional.ofNullable(imageUrl);
}

/**
Expand All @@ -62,16 +54,4 @@ public String getImageUrl() {
public Optional<String> getCaption() {
return Optional.ofNullable(caption);
}

/**
* Set the short description for the image
*
* @param caption
* A short image description
* @return this image
*/
public ChecksImage withCaption(final String caption) {
this.caption = requireNonNull(caption);
return this;
}
}
25 changes: 6 additions & 19 deletions src/main/java/io/jenkins/plugins/checks/api/ChecksOutput.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import static java.util.Objects.*;

Expand Down Expand Up @@ -74,8 +73,8 @@ public static class ChecksOutputBuilder {
*
*/
public ChecksOutputBuilder() {
this.annotations = Collections.emptyList();
this.images = Collections.emptyList();
this.annotations = new ArrayList<>();
this.images = new ArrayList<>();
}

/**
Expand Down Expand Up @@ -130,9 +129,7 @@ public ChecksOutputBuilder withText(final String text) {
* @return this builder
*/
public ChecksOutputBuilder withAnnotations(final List<ChecksAnnotation> annotations) {
this.annotations = requireNonNull(annotations).stream()
.map(ChecksAnnotation::new)
.collect(Collectors.toList());
this.annotations = new ArrayList<>(requireNonNull(annotations));
return this;
}

Expand All @@ -144,11 +141,7 @@ public ChecksOutputBuilder withAnnotations(final List<ChecksAnnotation> annotati
* @return this builder
*/
public ChecksOutputBuilder addAnnotation(final ChecksAnnotation annotation) {
requireNonNull(annotation);
if (annotations == Collections.EMPTY_LIST) {
annotations = new ArrayList<>();
}
annotations.add(new ChecksAnnotation(annotation));
annotations.add(new ChecksAnnotation(requireNonNull(annotation)));
return this;
}

Expand All @@ -159,9 +152,7 @@ public ChecksOutputBuilder addAnnotation(final ChecksAnnotation annotation) {
* @return this builder
*/
public ChecksOutputBuilder withImages(final List<ChecksImage> images) {
this.images = requireNonNull(images).stream()
.map(ChecksImage::new)
.collect(Collectors.toList());
this.images = new ArrayList<>(requireNonNull(images));
return this;
}

Expand All @@ -173,11 +164,7 @@ public ChecksOutputBuilder withImages(final List<ChecksImage> images) {
* @return this builder
*/
public ChecksOutputBuilder addImage(final ChecksImage image) {
requireNonNull(image);
if (images == Collections.EMPTY_LIST) {
images = new ArrayList<>();
}
images.add(new ChecksImage(image));
images.add(requireNonNull(image));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,16 @@ public List<Action> getActions() {
}

private Action getAction(final ChecksAction checksAction) {
return new Action(checksAction.getLabel(), checksAction.getDescription(), checksAction.getIdentifier());
return new Action(
checksAction.getLabel()
.orElseThrow(() ->
new IllegalArgumentException("Label of action is required but not provided")),
checksAction.getDescription()
.orElseThrow(() ->
new IllegalArgumentException("Description of action is required but not provided")),
checksAction.getIdentifier()
.orElseThrow(() ->
new IllegalArgumentException("Identifier of action is required but not provided")));
}

private Annotation getAnnotation(final ChecksAnnotation checksAnnotation) {
Expand All @@ -211,7 +220,11 @@ private Annotation getAnnotation(final ChecksAnnotation checksAnnotation) {
}

private Image getImage(final ChecksImage checksImage) {
return new Image(checksImage.getAlt(), checksImage.getImageUrl())
return new Image(
checksImage.getAlt()
.orElseThrow(() -> new IllegalArgumentException("alt of image is required but not provided.")),
checksImage.getImageUrl()
.orElseThrow(() -> new IllegalArgumentException("url of image is required but not provided.")))
.withCaption(checksImage.getCaption().orElse(null));
}

Expand Down
17 changes: 3 additions & 14 deletions src/test/java/io/jenkins/plugins/checks/api/ChecksActionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,8 @@ void shouldConstructCorrectly() {
final ChecksAction action =
new ChecksAction("re-run", "re-run the Jenkins build", "re-run id");

assertThat(action.getLabel()).isEqualTo("re-run");
assertThat(action.getDescription()).isEqualTo("re-run the Jenkins build");
assertThat(action.getIdentifier()).isEqualTo("re-run id");
}

@Test
void shouldCopyConstructCorrectly() {
final ChecksAction action =
new ChecksAction("re-run", "re-run the Jenkins build", "re-run id");
final ChecksAction copied = new ChecksAction(action);

assertThat(copied.getLabel()).isEqualTo("re-run");
assertThat(copied.getDescription()).isEqualTo("re-run the Jenkins build");
assertThat(copied.getIdentifier()).isEqualTo("re-run id");
assertThat(action.getLabel()).isPresent().hasValue("re-run");
assertThat(action.getDescription()).isPresent().hasValue("re-run the Jenkins build");
assertThat(action.getIdentifier()).isPresent().hasValue("re-run id");
}
}
27 changes: 6 additions & 21 deletions src/test/java/io/jenkins/plugins/checks/api/ChecksImageTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,15 @@
import static io.jenkins.plugins.checks.api.ChecksImageAssert.*;

class ChecksImageTest {
final String ALT = "warnings-chart";
final String IMAGE_URL = "https://ci.jenkins.io/job/Plugins/job/warnings-ng-plugin/job/master/";
final String CAPTION = "charts generated by warning-ng-plugin";

@Test
void shouldConstructCorrectly() {
final ChecksImage image = new ChecksImage(ALT, IMAGE_URL)
.withCaption(CAPTION);
final ChecksImage image = new ChecksImage("warnings-chart",
"https://ci.jenkins.io/job/Plugins/job/warnings-ng-plugin/job/master/",
"charts generated by warning-ng-plugin");

assertThat(image)
.hasAlt(ALT)
.hasImageUrl(IMAGE_URL)
.hasCaption(Optional.of(CAPTION));
}

@Test
void shouldCopyConstructCorrectly() {
final ChecksImage image = new ChecksImage(ALT, IMAGE_URL)
.withCaption(CAPTION);
final ChecksImage copied = new ChecksImage(image);

assertThat(copied)
.hasAlt(ALT)
.hasImageUrl(IMAGE_URL)
.hasCaption(Optional.of(CAPTION));
.hasAlt(Optional.of("warnings-chart"))
.hasImageUrl(Optional.of("https://ci.jenkins.io/job/Plugins/job/warnings-ng-plugin/job/master/"))
.hasCaption(Optional.of("charts generated by warning-ng-plugin"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ private List<ChecksAnnotation> createAnnotations() {

private List<ChecksImage> createImages() {
final List<ChecksImage> images = new ArrayList<>();
images.add(new ChecksImage("image_1", "https://www.image_1.com"));
images.add(new ChecksImage("image_2", "https://www.image_2.com"));
images.add(new ChecksImage("image_1", "https://www.image_1.com", null));
images.add(new ChecksImage("image_2", "https://www.image_2.com", null));
return images;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ void shouldPublishGitHubCheckRunCorrectly() throws IOException {
))
.withImages(Collections.singletonList(
new ChecksImage("Jenkins",
"https://ci.jenkins.io/static/cd5757a8/images/jenkins-header-logo-v2.svg")
.withCaption("Jenkins Symbol")
"https://ci.jenkins.io/static/cd5757a8/images/jenkins-header-logo-v2.svg",
"Jenkins Symbol")
))
.build())
.withActions(Collections.singletonList(
Expand Down

0 comments on commit 58d74e2

Please sign in to comment.