Skip to content

Commit

Permalink
Fixed #268
Browse files Browse the repository at this point in the history
 o Prevent to produce an NPE if getVersion() called
   before the first communication. So we call isRunning()
   which makes the first communication and will transfer
   http headers which contain the version which will be read
   by JenkinsHttpClient.
  • Loading branch information
khmarbaise committed Jul 20, 2017
1 parent 49be530 commit 196a94e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -103,9 +102,10 @@ public boolean isRunning() {
* @return {@link JenkinsVersion}
*/
public JenkinsVersion getVersion() {
if (client.getJenkinsVersion().isEmpty()) {
if (!client.isJenkinsVersionSet()) {
// Force a request to get at least once
// HttpHeader
// HttpHeader. The header contains the version
// information.
isRunning();
}
JenkinsVersion jv = new JenkinsVersion(client.getJenkinsVersion());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public class JenkinsHttpClient {

private String jenkinsVersion;

public final static String EMPTY_VERSION = "UNKNOWN";

/**
* Create an unauthenticated Jenkins HTTP client
*
Expand All @@ -82,7 +84,7 @@ public JenkinsHttpClient(URI uri, CloseableHttpClient client) {
this.client = client;
this.httpResponseValidator = new HttpResponseValidator();
// this.contentExtractor = new HttpResponseContentExtractor();
this.jenkinsVersion = null;
this.jenkinsVersion = EMPTY_VERSION;
LOGGER.debug("uri={}", uri.toString());
}

Expand Down Expand Up @@ -496,6 +498,16 @@ public String getJenkinsVersion() {
return this.jenkinsVersion;
}

/**
* Check to see if the jenkins version has been set
* to something different than the initialization value
* from the constructor. This means there has never been made
* a communication with the Jenkins server.
* @return true if jenkinsVersion has been set by communication, false otherwise.
*/
public boolean isJenkinsVersionSet() {
return !EMPTY_VERSION.equals( this.jenkinsVersion );
}
private void getJenkinsVersionFromHeader(HttpResponse response) {
Header[] headers = response.getHeaders("X-Jenkins");
if (headers.length == 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,16 @@
import com.offbytwo.jenkins.model.View;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class JenkinsServerTest extends BaseUnitTest {

private JenkinsHttpClient client = mock(JenkinsHttpClient.class);
private JenkinsServer server = new JenkinsServer(client);
private MainView mainView = new MainView(new Job("Hello", "http://localhost/job/Hello/"));

public JenkinsServerTest() throws Exception {
}

@Before
public void setUp() throws Exception {
given(client.get("/", MainView.class)).willReturn(mainView);
Expand Down Expand Up @@ -314,6 +313,21 @@ public void testGetJobXmls() throws Exception {
shouldGetJobXml("HeLLo");
}

@Test
public void getVersionShouldNotFailWithNPE()
throws Exception
{
when (client.get( "/" )).thenReturn( "TheAnswer");
when (client.getJenkinsVersion()).thenReturn( "1.23");

JenkinsServer server = new JenkinsServer( client);
server.getVersion();
verify( client, times( 1 )).isJenkinsVersionSet();
verify( client, times( 1 )).get( "/" );
verify( client, times( 1 )).getJenkinsVersion();

}

private void shouldGetFolderJobs(String... jobNames) throws IOException {
// given
String path = "http://localhost/jobs/someFolder/";
Expand Down

0 comments on commit 196a94e

Please sign in to comment.