From 8cc83f8f5db232254801f497adc9b03851e6c9dd Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Fri, 22 Jan 2021 16:52:41 -0800 Subject: [PATCH 1/3] close the closable response. --- .../ab/config/HttpProjectConfigManager.java | 16 +++++++++++--- .../config/HttpProjectConfigManagerTest.java | 22 ++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index afe7db451..5582af35d 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -24,6 +24,7 @@ import com.optimizely.ab.notification.NotificationCenter; import org.apache.http.*; import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.util.EntityUtils; import org.slf4j.Logger; @@ -81,7 +82,7 @@ public String getLastModified() { return datafileLastModified; } - public String getDatafileFromResponse(HttpResponse response) throws NullPointerException, IOException { + public String getDatafileFromResponse(CloseableHttpResponse response) throws NullPointerException, IOException { StatusLine statusLine = response.getStatusLine(); if (statusLine == null) { @@ -116,10 +117,10 @@ static ProjectConfig parseProjectConfig(String datafile) throws ConfigParseExcep @Override protected ProjectConfig poll() { HttpGet httpGet = createHttpRequest(); - + CloseableHttpResponse response = null; logger.debug("Fetching datafile from: {}", httpGet.getURI()); try { - HttpResponse response = httpClient.execute(httpGet); + response = httpClient.execute(httpGet); String datafile = getDatafileFromResponse(response); if (datafile == null) { return null; @@ -128,6 +129,15 @@ protected ProjectConfig poll() { } catch (ConfigParseException | IOException e) { logger.error("Error fetching datafile", e); } + finally { + if (response != null) { + try { + response.close(); + } catch (IOException e) { + logger.warn(e.getLocalizedMessage()); + } + } + } return null; } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index 43c0d3c31..644e50bb4 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -36,6 +36,7 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import java.io.IOException; import java.net.URI; import java.util.concurrent.TimeUnit; @@ -48,6 +49,17 @@ @RunWith(MockitoJUnitRunner.class) public class HttpProjectConfigManagerTest { + class MyResponse extends BasicHttpResponse implements CloseableHttpResponse { + + public MyResponse(ProtocolVersion protocolVersion, Integer status, String body) { + super(protocolVersion, status, body); + } + + @Override + public void close() throws IOException { + + } + } @Mock private OptimizelyHttpClient mockHttpClient; @@ -246,7 +258,7 @@ public void testInvalidBlockingTimeout() { @Ignore public void testGetDatafileHttpResponse2XX() throws Exception { String modifiedStamp = "Wed, 24 Apr 2019 07:07:07 GMT"; - HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 200, "TEST"); + CloseableHttpResponse getResponse = new MyResponse(new ProtocolVersion("TEST", 0, 0), 200, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); getResponse.setHeader(HttpHeaders.LAST_MODIFIED, modifiedStamp); @@ -260,7 +272,7 @@ public void testGetDatafileHttpResponse2XX() throws Exception { @Test(expected = ClientProtocolException.class) public void testGetDatafileHttpResponse3XX() throws Exception { - HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 300, "TEST"); + CloseableHttpResponse getResponse = new MyResponse(new ProtocolVersion("TEST", 0, 0), 300, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); projectConfigManager.getDatafileFromResponse(getResponse); @@ -268,7 +280,7 @@ public void testGetDatafileHttpResponse3XX() throws Exception { @Test public void testGetDatafileHttpResponse304() throws Exception { - HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 304, "TEST"); + CloseableHttpResponse getResponse = new MyResponse(new ProtocolVersion("TEST", 0, 0), 304, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); String datafile = projectConfigManager.getDatafileFromResponse(getResponse); @@ -277,7 +289,7 @@ public void testGetDatafileHttpResponse304() throws Exception { @Test(expected = ClientProtocolException.class) public void testGetDatafileHttpResponse4XX() throws Exception { - HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 400, "TEST"); + CloseableHttpResponse getResponse = new MyResponse(new ProtocolVersion("TEST", 0, 0), 400, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); projectConfigManager.getDatafileFromResponse(getResponse); @@ -285,7 +297,7 @@ public void testGetDatafileHttpResponse4XX() throws Exception { @Test(expected = ClientProtocolException.class) public void testGetDatafileHttpResponse5XX() throws Exception { - HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 500, "TEST"); + CloseableHttpResponse getResponse = new MyResponse(new ProtocolVersion("TEST", 0, 0), 500, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); projectConfigManager.getDatafileFromResponse(getResponse); From 2a373fba3fe45b222713f9c4dd1971ba3ce389f5 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 27 Jan 2021 09:55:40 -0800 Subject: [PATCH 2/3] fix spotbug error --- .../com/optimizely/ab/config/HttpProjectConfigManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index 644e50bb4..fb76a27b7 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -49,7 +49,7 @@ @RunWith(MockitoJUnitRunner.class) public class HttpProjectConfigManagerTest { - class MyResponse extends BasicHttpResponse implements CloseableHttpResponse { + static class MyResponse extends BasicHttpResponse implements CloseableHttpResponse { public MyResponse(ProtocolVersion protocolVersion, Integer status, String body) { super(protocolVersion, status, body); From e713952f3ed15cae63f144d6f1f77161f90f3e3e Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Wed, 3 Feb 2021 15:09:22 -0800 Subject: [PATCH 3/3] reset to HttpResponse agrument for public compatibility --- .../java/com/optimizely/ab/config/HttpProjectConfigManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index 5582af35d..eb43e67a5 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -82,7 +82,7 @@ public String getLastModified() { return datafileLastModified; } - public String getDatafileFromResponse(CloseableHttpResponse response) throws NullPointerException, IOException { + public String getDatafileFromResponse(HttpResponse response) throws NullPointerException, IOException { StatusLine statusLine = response.getStatusLine(); if (statusLine == null) {