Skip to content

Commit 1678223

Browse files
authored
Merge pull request #299 from dellgreen/jenkins-client-298
Fixed #298
2 parents 3d95998 + c4c8c4b commit 1678223

File tree

6 files changed

+92
-2
lines changed

6 files changed

+92
-2
lines changed

ReleaseNotes.md

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
## Release 0.3.8 (NOT RELEASED YET)
44

5+
6+
* [Fixed Issue 298][issue-298]
7+
8+
Added Closeable support to JenkinsServer and JenkinsHttpClient so that
9+
underlying resources can be cleaned up when instances no longer required
10+
11+
512
* [JENKINS-46472](https://issues.jenkins-ci.org/browse/JENKINS-46472)
613

714
Added ability to modify offline cause for offline computers.

jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,12 @@
4343
import com.offbytwo.jenkins.model.QueueItem;
4444
import com.offbytwo.jenkins.model.QueueReference;
4545
import com.offbytwo.jenkins.model.View;
46+
import java.io.Closeable;
4647

4748
/**
4849
* The main starting point for interacting with a Jenkins server.
4950
*/
50-
public class JenkinsServer {
51+
public class JenkinsServer implements Closeable {
5152
private final Logger LOGGER = LoggerFactory.getLogger(getClass());
5253

5354
private final JenkinsHttpClient client;
@@ -882,6 +883,18 @@ public void renameJob(FolderJob folder, String oldJobName, String newJobName, Bo
882883
+ "/doRename?newName=" + EncodingUtils.encodeParam(newJobName),
883884
crumbFlag);
884885
}
886+
887+
888+
889+
/**
890+
* Closes underlying resources.
891+
* Closed instances should no longer be used
892+
* Closing an already closed instance has no side effects
893+
*/
894+
@Override
895+
public void close() {
896+
client.close();
897+
}
885898

886899

887900
}

jenkins-client/src/main/java/com/offbytwo/jenkins/client/JenkinsHttpClient.java

+24-1
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@
4949
import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
5050
import com.offbytwo.jenkins.client.util.ResponseUtils;
5151
import com.offbytwo.jenkins.client.util.UrlUtils;
52+
import java.io.Closeable;
5253
import static org.apache.commons.lang.StringUtils.isNotBlank;
5354

5455
//import com.offbytwo.jenkins.client.util.HttpResponseContentExtractor;
5556

56-
public class JenkinsHttpClient {
57+
public class JenkinsHttpClient implements Closeable {
5758
private final Logger LOGGER = LoggerFactory.getLogger(getClass());
5859

5960
private URI uri;
@@ -498,7 +499,27 @@ public boolean isJenkinsVersionSet() {
498499
return !EMPTY_VERSION.equals( this.jenkinsVersion );
499500
}
500501

502+
503+
504+
505+
/**
506+
* Closes underlying resources.
507+
* Any I/O errors whilst closing are logged with debug log level
508+
* Closed instances should no longer be used
509+
* Closing an already closed instance has no side effects
510+
*/
511+
@Override
512+
public void close() {
513+
try {
514+
client.close();
515+
} catch (final IOException ex) {
516+
LOGGER.debug("I/O exception closing client", ex);
517+
}
518+
}
519+
501520

521+
522+
502523
private void releaseConnection(HttpRequestBase httpRequestBase) {
503524
httpRequestBase.releaseConnection();
504525
}
@@ -524,4 +545,6 @@ protected HttpContext getLocalContext() {
524545
protected void setLocalContext(HttpContext localContext) {
525546
this.localContext = localContext;
526547
}
548+
549+
527550
}

jenkins-client/src/test/java/com/offbytwo/jenkins/JenkinsServerTest.java

+15
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.offbytwo.jenkins.model.JobWithDetails;
3333
import com.offbytwo.jenkins.model.MainView;
3434
import com.offbytwo.jenkins.model.View;
35+
import java.net.URI;
3536

3637
import static org.mockito.Mockito.mock;
3738
import static org.mockito.Mockito.times;
@@ -328,6 +329,20 @@ public void getVersionShouldNotFailWithNPE()
328329

329330
}
330331

332+
333+
@Test(expected=IllegalStateException.class)
334+
public void testClose() throws Exception {
335+
final String uri = "http://localhost/jenkins";
336+
JenkinsServer srv = new JenkinsServer(new URI(uri));
337+
srv.close();
338+
srv.close(); //check multiple calls yield no errors
339+
srv.getComputers();
340+
}
341+
342+
343+
344+
345+
331346
private void shouldGetFolderJobs(String... jobNames) throws IOException {
332347
// given
333348
String path = "http://localhost/jobs/someFolder/";

jenkins-client/src/test/java/com/offbytwo/jenkins/client/JenkinsHttpClientTest.java

+10
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,16 @@ public void testGet_String() throws Exception {
5151
final String s = jclient.get("job/someJob");
5252
assertEquals("someJson", s);
5353
}
54+
55+
56+
57+
@Test(expected=IllegalStateException.class)
58+
public void testClose() throws Exception {
59+
final JenkinsHttpClient jclient = new JenkinsHttpClient(new URI(URI));
60+
jclient.close();
61+
jclient.close(); //check multiple calls yield no errors
62+
jclient.get("job/someJob");
63+
}
5464

5565

5666
}

jenkins-client/src/test/java/com/offbytwo/jenkins/integration/JenkinsServerIT.java

+22
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,26 @@ public void testUpdateJob() throws Exception {
183183
String confirmXml = server.getJobXml(JENKINS_TEST_JOB);
184184
assertTrue(confirmXml.contains(description));
185185
}
186+
187+
188+
@Test(expected=IllegalStateException.class)
189+
public void testClose_ReuseAfterClosed() throws Exception {
190+
final FreeStyleProject proj = jenkinsRule.getInstance().createProject(
191+
FreeStyleProject.class, JENKINS_TEST_JOB);
192+
final Map<String, Job> jobs = server.getJobs();
193+
assertNotNull(jobs.get(proj.getName()));
194+
server.close();
195+
server.getJobs();
196+
}
197+
198+
199+
@Test
200+
public void testClose_CloseMultipleTimes() throws Exception {
201+
final FreeStyleProject proj = jenkinsRule.getInstance().createProject(
202+
FreeStyleProject.class, JENKINS_TEST_JOB);
203+
final Map<String, Job> jobs = server.getJobs();
204+
assertNotNull(jobs.get(proj.getName()));
205+
server.close();
206+
server.close();
207+
}
186208
}

0 commit comments

Comments
 (0)