Fixed jenkinsci#268

khmarbaise · khmarbaise · commit e16dc93a4f8e · 2017-07-20T19:50:55.000+02:00
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.
diff --git a/ReleaseNotes.md b/ReleaseNotes.md
@@ -2,7 +2,11 @@
 
 ## Release 0.3.8 (NOT RELEASED YET)
 
-* [Fixed Issue 98][issue-98]
+ * [Fixed Issue 268][issue-268]
+  
+   NullPointerException is thrown unless isRunning() is called first.
+ 
+ * [Fixed Issue 98][issue-98]
    
    Splitting fix made for jacoco reports from Jenkins #98.
    Thanks to Shah, Prince.
@@ -1006,6 +1010,7 @@ TestReport testReport = mavenJob.getLastSuccessfulBuild().getTestReport();
 [issue-220]: https://github.com/jenkinsci/java-client-api/issues/220
 [issue-222]: https://github.com/jenkinsci/java-client-api/issues/222
 [issue-244]: https://github.com/jenkinsci/java-client-api/issues/244
+[issue-268]: https://github.com/jenkinsci/java-client-api/issues/268
 [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/src/main/java/com/offbytwo/jenkins/JenkinsServer.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java
@@ -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;
@@ -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());
diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/client/JenkinsHttpClient.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/client/JenkinsHttpClient.java
@@ -66,6 +66,8 @@ public class JenkinsHttpClient {
 
     private String jenkinsVersion;
 
+    public final static String EMPTY_VERSION = "UNKNOWN";
+    
     /**
      * Create an unauthenticated Jenkins HTTP client
      *
@@ -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());
     }
 
@@ -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) {
diff --git a/jenkins-client/src/test/java/com/offbytwo/jenkins/JenkinsServerTest.java b/jenkins-client/src/test/java/com/offbytwo/jenkins/JenkinsServerTest.java
@@ -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);
@@ -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/";
