Skip to content

Commit a0feba2

Browse files
khmarbaisekhmarbaise
committed
o Fixed implementation of BuildWithDetails.getCauses() to prevent NPE's.
1 parent 1ef87b6 commit a0feba2

5 files changed

Lines changed: 119 additions & 26 deletions

File tree

‎ReleaseNotes.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
* maven-resources-plugin to 3.0.0
2222
* maven-jar-plugin to 3.0.0
2323

24+
[Fixed issue 172][issue-172]
25+
26+
The implementation `BuildWithDetails.getCauses()` could cause an
27+
NPE which now has been imroved to prevent it. Thanks for hint
28+
which brought me to reconsider the implementation.
29+
2430
[Fixed issue 162][issue-162]
2531

2632
Serveral issues fixed related to using logging framework
@@ -508,6 +514,7 @@ TestReport testReport = mavenJob.getLastSuccessfulBuild().getTestReport();
508514
[issue-162]: https://github.com/jenkinsci/java-client-api/issues/162
509515
[issue-165]: https://github.com/jenkinsci/java-client-api/issues/165
510516
[issue-167]: https://github.com/jenkinsci/java-client-api/issues/167
517+
[issue-172]: https://github.com/jenkinsci/java-client-api/issues/172
511518
[pull-123]: https://github.com/jenkinsci/java-client-api/pull/123
512519
[pull-149]: https://github.com/jenkinsci/java-client-api/pull/149
513520
[pull-158]: https://github.com/jenkinsci/java-client-api/pull/158

‎jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public final class Constant {
1010
* At the moment it is solved by a profile in pom..but could that somehow
1111
* identified by docker itself ?
1212
*/
13-
public static final URI JENKINS_URI = URI.create(System.getProperty("docker.container.network"));
13+
// public static final URI JENKINS_URI = URI.create(System.getProperty("docker.container.network"));
14+
public static final URI JENKINS_URI = URI.create("http://192.168.99.100:8080/");
1415

1516
}

‎jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/NoExecutorStartedGetJobDetailsIT.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package com.offbytwo.jenkins.integration;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.io.IOException;
6+
import java.util.List;
7+
8+
import org.testng.annotations.BeforeMethod;
9+
import org.testng.annotations.Test;
10+
11+
import com.offbytwo.jenkins.model.BuildCause;
12+
import com.offbytwo.jenkins.model.BuildWithDetails;
13+
import com.offbytwo.jenkins.model.JobWithDetails;
14+
15+
@Test( groups = { Groups.NO_EXECUTOR_GROUP } )
16+
public class NoExecutorStartedGetJobDetailsIT
17+
extends AbstractJenkinsIntegrationCase
18+
{
19+
20+
private JobWithDetails job;
21+
22+
@BeforeMethod
23+
public void beforeMethod()
24+
throws IOException
25+
{
26+
job = jenkinsServer.getJob( "test" );
27+
}
28+
29+
@Test
30+
public void shouldCheckTheBuildCause() throws IOException
31+
{
32+
BuildWithDetails details = job.getFirstBuild().details();
33+
List<BuildCause> causes = details.getCauses();
34+
assertThat(causes).hasSize(1);
35+
BuildCause buildCause = causes.get(0);
36+
37+
assertThat(buildCause.getShortDescription()).isEqualTo("Started by user anonymous");
38+
assertThat(buildCause.getUserName()).isEqualTo("anonymous");
39+
assertThat(buildCause.getUpstreamBuild()).isEqualTo(0);
40+
assertThat(buildCause.getUpstreamProject()).isNull();
41+
assertThat(buildCause.getUserId()).isNull();
42+
43+
}
44+
45+
46+
}

‎jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildCause.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ public class BuildCause {
1414

1515
public BuildCause() {
1616
this.upstreamBuild = new Integer(0);
17+
//TODO: Think about initialization of the other
18+
// attributes as well.
19+
// userId = StringConstant.EMPTY;
1720
}
1821

1922
public String getShortDescription() {

‎jenkins-client/src/main/java/com/offbytwo/jenkins/model/BuildWithDetails.java‎

Lines changed: 61 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package com.offbytwo.jenkins.model;
88

99
import com.google.common.base.Predicate;
10+
import com.google.common.base.Strings;
1011

1112
import java.io.IOException;
1213
import java.io.InputStream;
@@ -16,23 +17,29 @@
1617

1718
import static com.google.common.collect.Collections2.filter;
1819

20+
/**
21+
* This class represents build information with
22+
* details about what has been done like
23+
* duration start and of course the build result.
24+
*
25+
*/
1926
public class BuildWithDetails extends Build {
2027

21-
List actions; // Should be improved.
22-
boolean building;
23-
String description;
24-
long duration;
25-
long estimatedDuration;
26-
String fullDisplayName;
27-
String id;
28-
long timestamp;
29-
BuildResult result;
30-
List<Artifact> artifacts;
31-
String consoleOutputText;
32-
String consoleOutputHtml;
33-
BuildChangeSet changeSet;
34-
String builtOn;
35-
List<BuildChangeSetAuthor> culprits;
28+
private List actions; // TODO: Should be improved.
29+
private boolean building;
30+
private String description;
31+
private long duration;
32+
private long estimatedDuration;
33+
private String fullDisplayName;
34+
private String id;
35+
private long timestamp;
36+
private BuildResult result;
37+
private List<Artifact> artifacts;
38+
private String consoleOutputText;
39+
private String consoleOutputHtml;
40+
private BuildChangeSet changeSet;
41+
private String builtOn;
42+
private List<BuildChangeSetAuthor> culprits;
3643

3744
public List<Artifact> getArtifacts() {
3845
return artifacts;
@@ -61,21 +68,51 @@ public boolean apply(Map<String, Object> action) {
6168
.get("causes");
6269
for (Map<String, Object> cause : causes_blob) {
6370

64-
BuildCause cause_object = new BuildCause();
65-
cause_object.setShortDescription((String) cause.get("shortDescription"));
66-
cause_object.setUpstreamBuild((Integer) cause.get("upstreamBuild"));
67-
cause_object.setUpstreamProject((String) cause.get("upstreamProject"));
68-
cause_object.setUpstreamUrl((String) cause.get("upstreamUrl"));
69-
cause_object.setUserId((String) cause.get("userId"));
70-
cause_object.setUserName((String) cause.get("userName"));
71+
BuildCause convertToBuildCause = convertToBuildCause(cause);
7172

72-
result.add(cause_object);
73+
result.add(convertToBuildCause);
7374
}
7475
}
7576

7677
return result;
7778
}
7879

80+
private BuildCause convertToBuildCause(Map<String, Object> cause) {
81+
BuildCause cause_object = new BuildCause();
82+
83+
//TODO: Think about it. Can this be done more simpler?
84+
String description = (String) cause.get("shortDescription");
85+
if (!Strings.isNullOrEmpty(description)) {
86+
cause_object.setShortDescription(description);
87+
}
88+
89+
Integer upstreamBuild = (Integer) cause.get("upstreamBuild");
90+
if (upstreamBuild != null) {
91+
cause_object.setUpstreamBuild(upstreamBuild);
92+
}
93+
94+
String upstreamProject = (String) cause.get("upstreamProject");
95+
if (!Strings.isNullOrEmpty(upstreamProject)) {
96+
cause_object.setUpstreamProject(upstreamProject);
97+
}
98+
99+
String upstreamUrl = (String) cause.get("upstreamUrl");
100+
if (!Strings.isNullOrEmpty(upstreamProject)) {
101+
cause_object.setUpstreamUrl(upstreamUrl);
102+
}
103+
104+
String userId = (String) cause.get("userId");
105+
if (!Strings.isNullOrEmpty(userId)) {
106+
cause_object.setUserId(userId);
107+
}
108+
109+
String userName = (String) cause.get("userName");
110+
if (!Strings.isNullOrEmpty(userName)) {
111+
cause_object.setUserName(userName);
112+
}
113+
return cause_object;
114+
}
115+
79116
public String getDescription() {
80117
return description;
81118
}
@@ -166,8 +203,7 @@ public void setCulprits(List<BuildChangeSetAuthor> culprits) {
166203

167204
public InputStream downloadArtifact(Artifact a) throws IOException, URISyntaxException {
168205
// We can't just put the artifact's relative path at the end of the url
169-
// string,
170-
// as there could be characters that need to be escaped.
206+
// string, as there could be characters that need to be escaped.
171207
URI uri = new URI(getUrl());
172208
String artifactPath = uri.getPath() + "artifact/" + a.getRelativePath();
173209
URI artifactUri = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), artifactPath, "",

0 commit comments

Comments
 (0)