From a0feba25138cc99ac6917bb407f17434afa8448c Mon Sep 17 00:00:00 2001 From: Karl Heinz Marbaise Date: Sun, 24 Jul 2016 19:34:56 +0200 Subject: [PATCH] Fixed #172 o Fixed implementation of BuildWithDetails.getCauses() to prevent NPE's. --- ReleaseNotes.md | 7 ++ .../jenkins/integration/Constant.java | 3 +- .../NoExecutorStartedGetJobDetailsIT.java | 46 ++++++++++ .../offbytwo/jenkins/model/BuildCause.java | 3 + .../jenkins/model/BuildWithDetails.java | 86 +++++++++++++------ 5 files changed, 119 insertions(+), 26 deletions(-) create mode 100644 jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/NoExecutorStartedGetJobDetailsIT.java diff --git a/ReleaseNotes.md b/ReleaseNotes.md index e3a878ca..1234fa50 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -21,6 +21,12 @@ * maven-resources-plugin to 3.0.0 * maven-jar-plugin to 3.0.0 + [Fixed issue 172][issue-172] + + The implementation `BuildWithDetails.getCauses()` could cause an + NPE which now has been imroved to prevent it. Thanks for hint + which brought me to reconsider the implementation. + [Fixed issue 162][issue-162] Serveral issues fixed related to using logging framework @@ -508,6 +514,7 @@ TestReport testReport = mavenJob.getLastSuccessfulBuild().getTestReport(); [issue-162]: https://github.com/jenkinsci/java-client-api/issues/162 [issue-165]: https://github.com/jenkinsci/java-client-api/issues/165 [issue-167]: https://github.com/jenkinsci/java-client-api/issues/167 +[issue-172]: https://github.com/jenkinsci/java-client-api/issues/172 [pull-123]: https://github.com/jenkinsci/java-client-api/pull/123 [pull-149]: https://github.com/jenkinsci/java-client-api/pull/149 [pull-158]: https://github.com/jenkinsci/java-client-api/pull/158 diff --git a/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java index 066e2a87..c8b2dd6b 100644 --- a/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java +++ b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java @@ -10,6 +10,7 @@ public final class Constant { * At the moment it is solved by a profile in pom..but could that somehow * identified by docker itself ? */ - public static final URI JENKINS_URI = URI.create(System.getProperty("docker.container.network")); +// public static final URI JENKINS_URI = URI.create(System.getProperty("docker.container.network")); + public static final URI JENKINS_URI = URI.create("http://192.168.99.100:8080/"); } diff --git a/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/NoExecutorStartedGetJobDetailsIT.java b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/NoExecutorStartedGetJobDetailsIT.java new file mode 100644 index 00000000..5b169b71 --- /dev/null +++ b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/NoExecutorStartedGetJobDetailsIT.java @@ -0,0 +1,46 @@ +package com.offbytwo.jenkins.integration; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.util.List; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.offbytwo.jenkins.model.BuildCause; +import com.offbytwo.jenkins.model.BuildWithDetails; +import com.offbytwo.jenkins.model.JobWithDetails; + +@Test( groups = { Groups.NO_EXECUTOR_GROUP } ) +public class NoExecutorStartedGetJobDetailsIT + extends AbstractJenkinsIntegrationCase +{ + + private JobWithDetails job; + + @BeforeMethod + public void beforeMethod() + throws IOException + { + job = jenkinsServer.getJob( "test" ); + } + + @Test + public void shouldCheckTheBuildCause() throws IOException + { + BuildWithDetails details = job.getFirstBuild().details(); + List causes = details.getCauses(); + assertThat(causes).hasSize(1); + BuildCause buildCause = causes.get(0); + + assertThat(buildCause.getShortDescription()).isEqualTo("Started by user anonymous"); + assertThat(buildCause.getUserName()).isEqualTo("anonymous"); + assertThat(buildCause.getUpstreamBuild()).isEqualTo(0); + assertThat(buildCause.getUpstreamProject()).isNull(); + assertThat(buildCause.getUserId()).isNull(); + + } + + +} diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildCause.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildCause.java index 6490b4a7..30a9a98d 100644 --- a/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildCause.java +++ b/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildCause.java @@ -14,6 +14,9 @@ public class BuildCause { public BuildCause() { this.upstreamBuild = new Integer(0); + //TODO: Think about initialization of the other + // attributes as well. + // userId = StringConstant.EMPTY; } public String getShortDescription() { diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java index f9ec3c25..e16b9237 100644 --- a/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java +++ b/jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java @@ -7,6 +7,7 @@ package com.offbytwo.jenkins.model; import com.google.common.base.Predicate; +import com.google.common.base.Strings; import java.io.IOException; import java.io.InputStream; @@ -16,23 +17,29 @@ import static com.google.common.collect.Collections2.filter; +/** + * This class represents build information with + * details about what has been done like + * duration start and of course the build result. + * + */ public class BuildWithDetails extends Build { - List actions; // Should be improved. - boolean building; - String description; - long duration; - long estimatedDuration; - String fullDisplayName; - String id; - long timestamp; - BuildResult result; - List artifacts; - String consoleOutputText; - String consoleOutputHtml; - BuildChangeSet changeSet; - String builtOn; - List culprits; + private List actions; // TODO: Should be improved. + private boolean building; + private String description; + private long duration; + private long estimatedDuration; + private String fullDisplayName; + private String id; + private long timestamp; + private BuildResult result; + private List artifacts; + private String consoleOutputText; + private String consoleOutputHtml; + private BuildChangeSet changeSet; + private String builtOn; + private List culprits; public List getArtifacts() { return artifacts; @@ -61,21 +68,51 @@ public boolean apply(Map action) { .get("causes"); for (Map cause : causes_blob) { - BuildCause cause_object = new BuildCause(); - cause_object.setShortDescription((String) cause.get("shortDescription")); - cause_object.setUpstreamBuild((Integer) cause.get("upstreamBuild")); - cause_object.setUpstreamProject((String) cause.get("upstreamProject")); - cause_object.setUpstreamUrl((String) cause.get("upstreamUrl")); - cause_object.setUserId((String) cause.get("userId")); - cause_object.setUserName((String) cause.get("userName")); + BuildCause convertToBuildCause = convertToBuildCause(cause); - result.add(cause_object); + result.add(convertToBuildCause); } } return result; } + private BuildCause convertToBuildCause(Map cause) { + BuildCause cause_object = new BuildCause(); + + //TODO: Think about it. Can this be done more simpler? + String description = (String) cause.get("shortDescription"); + if (!Strings.isNullOrEmpty(description)) { + cause_object.setShortDescription(description); + } + + Integer upstreamBuild = (Integer) cause.get("upstreamBuild"); + if (upstreamBuild != null) { + cause_object.setUpstreamBuild(upstreamBuild); + } + + String upstreamProject = (String) cause.get("upstreamProject"); + if (!Strings.isNullOrEmpty(upstreamProject)) { + cause_object.setUpstreamProject(upstreamProject); + } + + String upstreamUrl = (String) cause.get("upstreamUrl"); + if (!Strings.isNullOrEmpty(upstreamProject)) { + cause_object.setUpstreamUrl(upstreamUrl); + } + + String userId = (String) cause.get("userId"); + if (!Strings.isNullOrEmpty(userId)) { + cause_object.setUserId(userId); + } + + String userName = (String) cause.get("userName"); + if (!Strings.isNullOrEmpty(userName)) { + cause_object.setUserName(userName); + } + return cause_object; + } + public String getDescription() { return description; } @@ -166,8 +203,7 @@ public void setCulprits(List culprits) { public InputStream downloadArtifact(Artifact a) throws IOException, URISyntaxException { // We can't just put the artifact's relative path at the end of the url - // string, - // as there could be characters that need to be escaped. + // string, as there could be characters that need to be escaped. URI uri = new URI(getUrl()); String artifactPath = uri.getPath() + "artifact/" + a.getRelativePath(); URI artifactUri = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), artifactPath, "",