Skip to content

Commit

Permalink
Fixed #172
Browse files Browse the repository at this point in the history
 o Fixed implementation of BuildWithDetails.getCauses() to
   prevent NPE's.
  • Loading branch information
khmarbaise committed Jul 24, 2016
1 parent 1ef87b6 commit a0feba2
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 26 deletions.
7 changes: 7 additions & 0 deletions ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/");

}
Original file line number Diff line number Diff line change
@@ -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<BuildCause> 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();

}


}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Artifact> artifacts;
String consoleOutputText;
String consoleOutputHtml;
BuildChangeSet changeSet;
String builtOn;
List<BuildChangeSetAuthor> 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<Artifact> artifacts;
private String consoleOutputText;
private String consoleOutputHtml;
private BuildChangeSet changeSet;
private String builtOn;
private List<BuildChangeSetAuthor> culprits;

public List<Artifact> getArtifacts() {
return artifacts;
Expand Down Expand Up @@ -61,21 +68,51 @@ public boolean apply(Map<String, Object> action) {
.get("causes");
for (Map<String, Object> 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<String, Object> 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;
}
Expand Down Expand Up @@ -166,8 +203,7 @@ public void setCulprits(List<BuildChangeSetAuthor> 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, "",
Expand Down

0 comments on commit a0feba2

Please sign in to comment.