[DURACOM-109] Fixed http connection leaks

dspace-api/src/main/java/org/dspace/app/sherpa/SHERPAService.java

@@ -17,9 +17,9 @@ import javax.annotation.PostConstruct;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.impl.client.CloseableHttpClient;
@@ -45,8 +45,6 @@ import org.springframework.cache.annotation.Cacheable;
  */
 public class SHERPAService {
 
-    private CloseableHttpClient client = null;
-
     private int maxNumberOfTries;
     private long sleepBetweenTimeouts;
     private int timeout = 5000;
@@ -59,15 +57,6 @@ public class SHERPAService {
     @Autowired
     ConfigurationService configurationService;
 
-    /**
-     * Create a new HTTP builder with sensible defaults in constructor
-     */
-    public SHERPAService() {
-        // httpclient 4.3+ doesn't appear to have any sensible defaults any more. Setting conservative defaults as
-        // not to hammer the SHERPA service too much.
-        client = DSpaceHttpClientFactory.getInstance().buildWithoutAutomaticRetries(5);
-    }
-
     /**
      * Complete initialization of the Bean.
      */
@@ -128,14 +117,14 @@ public class SHERPAService {
                 timeout,
                 sleepBetweenTimeouts));
 
-            try {
+            try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
                 Thread.sleep(sleepBetweenTimeouts);
 
                 // Construct a default HTTP method (first result)
                 method = constructHttpGet(type, field, predicate, value, start, limit);
 
                 // Execute the method
-                HttpResponse response = client.execute(method);
+                CloseableHttpResponse response = client.execute(method);
                 int statusCode = response.getStatusLine().getStatusCode();
 
                 log.debug(response.getStatusLine().getStatusCode() + ": "
@@ -231,14 +220,14 @@ public class SHERPAService {
                 timeout,
                 sleepBetweenTimeouts));
 
-            try {
+            try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
                 Thread.sleep(sleepBetweenTimeouts);
 
                 // Construct a default HTTP method (first result)
                 method = constructHttpGet(type, field, predicate, value, start, limit);
 
                 // Execute the method
-                HttpResponse response = client.execute(method);
+                CloseableHttpResponse response = client.execute(method);
                 int statusCode = response.getStatusLine().getStatusCode();
 
                 log.debug(response.getStatusLine().getStatusCode() + ": "

dspace-api/src/main/java/org/dspace/app/util/WebAppServiceImpl.java

@@ -13,8 +13,8 @@ import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 
-import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpHead;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.logging.log4j.Logger;
@@ -78,7 +78,7 @@ public class WebAppServiceImpl implements WebAppService {
                 method = new HttpHead(app.getUrl());
                 int status;
                 try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-                    HttpResponse response = client.execute(method);
+                    CloseableHttpResponse response = client.execute(method);
                     status = response.getStatusLine().getStatusCode();
                 }
                 if (status != HttpStatus.SC_OK) {

dspace-api/src/main/java/org/dspace/authenticate/oidc/impl/OidcClientImpl.java

@@ -22,10 +22,11 @@ import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.entity.UrlEncodedFormEntity;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.client.methods.RequestBuilder;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.message.BasicNameValuePair;
 import org.dspace.app.client.DSpaceHttpClientFactory;
 import org.dspace.authenticate.oidc.OidcClient;
@@ -83,21 +84,17 @@ public class OidcClientImpl implements OidcClient {
     }
 
     private <T> T executeAndParseJson(HttpUriRequest httpUriRequest, Class<T> clazz) {
-
+        try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-        HttpClient client = DSpaceHttpClientFactory.getInstance().build();
-
             return executeAndReturns(() -> {
-
+                CloseableHttpResponse response = client.execute(httpUriRequest);
-            HttpResponse response = client.execute(httpUriRequest);
-
                 if (isNotSuccessfull(response)) {
                     throw new OidcClientException(getStatusCode(response), formatErrorMessage(response));
                 }
-
                 return objectMapper.readValue(getContent(response), clazz);
-
             });
-
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     private <T> T executeAndReturns(ThrowingSupplier<T, Exception> supplier) {

dspace-api/src/main/java/org/dspace/content/packager/AbstractMETSIngester.java

@@ -1312,9 +1312,8 @@ public abstract class AbstractMETSIngester extends AbstractPackageIngester {
         if (params.getBooleanProperty("manifestOnly", false)) {
             // NOTE: since we are only dealing with a METS manifest,
             // we will assume all external files are available via URLs.
-            try {
+            try (CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build()) {
                 // attempt to open a connection to given URL
-                CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build();
                 CloseableHttpResponse httpResponse = httpClient.execute(new HttpGet(path));
 
                 // open stream to access file contents

dspace-api/src/main/java/org/dspace/ctask/general/BasicLinkChecker.java

@@ -139,9 +139,8 @@ public class BasicLinkChecker extends AbstractCurationTask {
      * @return The HTTP response code (e.g. 200 / 301 / 404 / 500)
      */
     protected int getResponseStatus(String url, int redirects) {
-        try {
         RequestConfig config = RequestConfig.custom().setRedirectsEnabled(true).build();
-            CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().buildWithRequestConfig(config);
+        try (CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().buildWithRequestConfig(config)) {
             CloseableHttpResponse httpResponse = httpClient.execute(new HttpGet(url));
             int statusCode = httpResponse.getStatusLine().getStatusCode();
             int maxRedirect = configurationService.getIntProperty("curate.checklinks.max-redirect", 0);

dspace-api/src/main/java/org/dspace/external/OrcidRestConnector.java

@@ -12,9 +12,9 @@ import java.nio.charset.StandardCharsets;
 import java.util.Scanner;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.client.DSpaceHttpClientFactory;
@@ -39,7 +39,7 @@ public class OrcidRestConnector {
     }
 
     public InputStream get(String path, String accessToken) {
-        HttpResponse getResponse = null;
+        CloseableHttpResponse getResponse = null;
         InputStream result = null;
         path = trimSlashes(path);
 
@@ -49,10 +49,8 @@ public class OrcidRestConnector {
             httpGet.addHeader("Content-Type", "application/vnd.orcid+xml");
             httpGet.addHeader("Authorization","Bearer " + accessToken);
         }
-        try {
+        try (CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build()) {
-            HttpClient httpClient = DSpaceHttpClientFactory.getInstance().build();
             getResponse = httpClient.execute(httpGet);
-            //do not close this httpClient
             result = getResponse.getEntity().getContent();
         } catch (Exception e) {
             getGotError(e, fullPath);

dspace-api/src/main/java/org/dspace/iiif/IIIFApiQueryServiceImpl.java

@@ -38,8 +38,7 @@ public class IIIFApiQueryServiceImpl implements IIIFApiQueryService {
         int[] arr = new int[2];
         String path = IIIFSharedUtils.getInfoJsonPath(bitstream);
         BufferedReader in = null;
-        try {
+        try (CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build()) {
-            CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build();
             CloseableHttpResponse httpResponse = httpClient.execute(new HttpGet(path));
             in = new BufferedReader(new InputStreamReader(httpResponse.getEntity().getContent()));
             String inputLine;

dspace-api/src/main/java/org/dspace/license/CCLicenseConnectorServiceImpl.java

@@ -326,8 +326,7 @@ public class CCLicenseConnectorServiceImpl implements CCLicenseConnectorService,
     public Document retrieveLicenseRDFDoc(String licenseURI) throws IOException {
         String ccLicenseUrl = configurationService.getProperty("cc.api.rooturl");
         String issueUrl = ccLicenseUrl + "/details?license-uri=" + licenseURI;
-        try {
+        try (CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build()) {
-            CloseableHttpClient httpClient = DSpaceHttpClientFactory.getInstance().build();
             CloseableHttpResponse httpResponse = httpClient.execute(new HttpPost(issueUrl));
             // parsing document from input stream
             InputStream stream = httpResponse.getEntity().getContent();

dspace-api/src/main/java/org/dspace/orcid/client/OrcidClientImpl.java

@@ -35,11 +35,12 @@ import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.NameValuePair;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.entity.UrlEncodedFormEntity;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.client.methods.RequestBuilder;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.message.BasicNameValuePair;
 import org.dspace.app.client.DSpaceHttpClientFactory;
 import org.dspace.orcid.OrcidToken;
@@ -76,12 +77,9 @@ public class OrcidClientImpl implements OrcidClient {
 
     private final ObjectMapper objectMapper;
 
-    private final DSpaceHttpClientFactory httpClientFactory;
+    public OrcidClientImpl(OrcidConfiguration orcidConfiguration) {
-
-    public OrcidClientImpl(OrcidConfiguration orcidConfiguration, DSpaceHttpClientFactory httpClientFactory) {
         this.orcidConfiguration = orcidConfiguration;
         this.objectMapper = new ObjectMapper();
-        this.httpClientFactory = httpClientFactory;
     }
 
     private static Map<Class<?>, String> initializePathsMap() {
@@ -257,10 +255,8 @@ public class OrcidClientImpl implements OrcidClient {
     }
 
     private void executeSuccessful(HttpUriRequest httpUriRequest) {
-        try {
+        try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-            HttpClient client = httpClientFactory.build();
+            CloseableHttpResponse response = client.execute(httpUriRequest);
-            HttpResponse response = client.execute(httpUriRequest);
-
             if (isNotSuccessfull(response)) {
                 throw new OrcidClientException(
                     getStatusCode(response),
@@ -275,21 +271,17 @@ public class OrcidClientImpl implements OrcidClient {
     }
 
     private <T> T executeAndParseJson(HttpUriRequest httpUriRequest, Class<T> clazz) {
-
+        try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-        HttpClient client = httpClientFactory.build();
-
             return executeAndReturns(() -> {
-
+                CloseableHttpResponse response = client.execute(httpUriRequest);
-            HttpResponse response = client.execute(httpUriRequest);
-
                 if (isNotSuccessfull(response)) {
                     throw new OrcidClientException(getStatusCode(response), formatErrorMessage(response));
                 }
-
                 return objectMapper.readValue(response.getEntity().getContent(), clazz);
-
             });
-
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     /**
@@ -304,44 +296,37 @@ public class OrcidClientImpl implements OrcidClient {
      * @throws OrcidClientException if the incoming response is not successfull
      */
     private <T> T executeAndUnmarshall(HttpUriRequest httpUriRequest, boolean handleNotFoundAsNull, Class<T> clazz) {
-
+        try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-        HttpClient client = httpClientFactory.build();
-
             return executeAndReturns(() -> {
-
+                CloseableHttpResponse response = client.execute(httpUriRequest);
-            HttpResponse response = client.execute(httpUriRequest);
-
                 if (handleNotFoundAsNull && isNotFound(response)) {
                     return null;
                 }
-
                 if (isNotSuccessfull(response)) {
                     throw new OrcidClientException(getStatusCode(response), formatErrorMessage(response));
                 }
-
                 return unmarshall(response.getEntity(), clazz);
-
             });
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     private OrcidResponse execute(HttpUriRequest httpUriRequest, boolean handleNotFoundAsNull) {
-        HttpClient client = httpClientFactory.build();
+        try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().build()) {
-
             return executeAndReturns(() -> {
-
+                CloseableHttpResponse response = client.execute(httpUriRequest);
-            HttpResponse response = client.execute(httpUriRequest);
-
                 if (handleNotFoundAsNull && isNotFound(response)) {
                     return new OrcidResponse(getStatusCode(response), null, getContent(response));
                 }
-
                 if (isNotSuccessfull(response)) {
                     throw new OrcidClientException(getStatusCode(response), formatErrorMessage(response));
                 }
-
                 return new OrcidResponse(getStatusCode(response), getPutCode(response), getContent(response));
-
             });
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     private <T> T executeAndReturns(ThrowingSupplier<T, Exception> supplier) {