From f9c14eb08cb58b58273ca79c65689228cdeb930a Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Wed, 9 Nov 2022 11:02:25 -0600 Subject: [PATCH] update to use same try-with-resources for CloseableHttpClient and CloseableHttpResponse --- .../module/api/actions/APICountAction.java | 21 +++-- .../module/api/actions/APIGetAction.java | 17 +++-- .../module/api/actions/APIInsertAction.java | 46 +++++------ .../module/api/actions/APIQueryAction.java | 76 +++++++++---------- .../module/api/actions/APIUpdateAction.java | 65 +++++++--------- .../backend/module/api/EasyPostApiTest.java | 30 +++++++- .../qqq/backend/module/api/TestUtils.java | 2 +- 7 files changed, 129 insertions(+), 128 deletions(-) diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APICountAction.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APICountAction.java index 1433ee8b..60e56469 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APICountAction.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APICountAction.java @@ -28,9 +28,9 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountOutput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; -import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -53,14 +53,11 @@ public class APICountAction extends AbstractAPIAction implements CountInterface QTableMetaData table = countInput.getTable(); preAction(countInput); - try + try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { QQueryFilter filter = countInput.getFilter(); String paramString = apiActionUtil.buildQueryStringForGet(filter, null, null, table.getFields()); - HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); - HttpClient client = httpClientBuilder.build(); - String url = apiActionUtil.buildTableUrl(table) + paramString; LOG.info("API URL: " + url); HttpGet request = new HttpGet(url); @@ -69,12 +66,14 @@ public class APICountAction extends AbstractAPIAction implements CountInterface apiActionUtil.setupContentTypeInRequest(request); apiActionUtil.setupAdditionalHeaders(request); - HttpResponse response = client.execute(request); - Integer count = apiActionUtil.processGetResponseForCount(table, response); + try(CloseableHttpResponse response = httpClient.execute(request)) + { + Integer count = apiActionUtil.processGetResponseForCount(table, response); - CountOutput rs = new CountOutput(); - rs.setCount(count); - return rs; + CountOutput rs = new CountOutput(); + rs.setCount(count); + return rs; + } } catch(Exception e) { diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIGetAction.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIGetAction.java index 78d99326..ccae3a18 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIGetAction.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIGetAction.java @@ -28,7 +28,7 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.get.GetInput; import com.kingsrook.qqq.backend.core.model.actions.tables.get.GetOutput; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; -import org.apache.http.HttpResponse; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -53,8 +53,7 @@ public class APIGetAction extends AbstractAPIAction implements GetInterface QTableMetaData table = getInput.getTable(); preAction(getInput); - HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); - try(CloseableHttpClient client = httpClientBuilder.build()) + try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { String urlSuffix = apiActionUtil.buildUrlSuffixForSingleRecordGet(getInput.getPrimaryKey()); @@ -65,12 +64,14 @@ public class APIGetAction extends AbstractAPIAction implements GetInterface apiActionUtil.setupContentTypeInRequest(request); apiActionUtil.setupAdditionalHeaders(request); - HttpResponse response = client.execute(request); - QRecord record = apiActionUtil.processSingleRecordGetResponse(table, response); + try(CloseableHttpResponse response = httpClient.execute(request)) + { + QRecord record = apiActionUtil.processSingleRecordGetResponse(table, response); - GetOutput rs = new GetOutput(); - rs.setRecord(record); - return rs; + GetOutput rs = new GetOutput(); + rs.setRecord(record); + return rs; + } } catch(Exception e) { diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIInsertAction.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIInsertAction.java index 9b69a342..5d0f59cd 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIInsertAction.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIInsertAction.java @@ -33,12 +33,11 @@ import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.SleepUtils; import com.kingsrook.qqq.backend.module.api.exceptions.RateLimitException; -import org.apache.http.HttpResponse; +import org.apache.http.HttpStatus; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; -import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -72,11 +71,8 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac preAction(insertInput); - HttpClientConnectionManager connectionManager = null; - try + try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { - connectionManager = new PoolingHttpClientConnectionManager(); - // todo - supports bulk post? for(QRecord record : insertInput.getRecords()) @@ -87,7 +83,7 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac ////////////////////////////////////////////////////////// insertInput.getAsyncJobCallback().incrementCurrent(); - postOneRecord(insertOutput, table, connectionManager, record); + postOneRecord(insertOutput, table, httpClient, record); if(insertInput.getRecords().size() > 1 && apiActionUtil.getMillisToSleepAfterEveryCall() > 0) { @@ -102,14 +98,6 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac LOG.warn("Error in API Insert for [" + table.getName() + "]", e); throw new QException("Error executing insert: " + e.getMessage(), e); } - finally - { - if(connectionManager != null) - { - connectionManager.shutdown(); - } - } - } @@ -117,7 +105,7 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac /******************************************************************************* ** *******************************************************************************/ - private void postOneRecord(InsertOutput insertOutput, QTableMetaData table, HttpClientConnectionManager connectionManager, QRecord record) throws RateLimitException + private void postOneRecord(InsertOutput insertOutput, QTableMetaData table, CloseableHttpClient httpClient, QRecord record) throws RateLimitException { int sleepMillis = apiActionUtil.getInitialRateLimitBackoffMillis(); int rateLimitsCaught = 0; @@ -125,7 +113,7 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac { try { - postOneTime(insertOutput, table, connectionManager, record); + postOneTime(insertOutput, table, httpClient, record); return; } catch(RateLimitException rle) @@ -151,9 +139,9 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac /******************************************************************************* ** *******************************************************************************/ - private void postOneTime(InsertOutput insertOutput, QTableMetaData table, HttpClientConnectionManager connectionManager, QRecord record) throws RateLimitException + private void postOneTime(InsertOutput insertOutput, QTableMetaData table, CloseableHttpClient httpClient, QRecord record) throws RateLimitException { - try(CloseableHttpClient client = HttpClients.custom().setConnectionManager(connectionManager).build()) + try { String url = apiActionUtil.buildTableUrl(table); HttpPost request = new HttpPost(url); @@ -163,15 +151,17 @@ public class APIInsertAction extends AbstractAPIAction implements InsertInterfac request.setEntity(apiActionUtil.recordToEntity(table, record)); - HttpResponse response = client.execute(request); - int statusCode = response.getStatusLine().getStatusCode(); - if(statusCode == 429) + try(CloseableHttpResponse response = httpClient.execute(request)) { - throw (new RateLimitException(EntityUtils.toString(response.getEntity()))); - } + int statusCode = response.getStatusLine().getStatusCode(); + if(statusCode == HttpStatus.SC_TOO_MANY_REQUESTS) + { + throw (new RateLimitException(EntityUtils.toString(response.getEntity()))); + } - QRecord outputRecord = apiActionUtil.processPostResponse(table, record, response); - insertOutput.addRecord(outputRecord); + QRecord outputRecord = apiActionUtil.processPostResponse(table, record, response); + insertOutput.addRecord(outputRecord); + } } catch(RateLimitException rle) { diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIQueryAction.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIQueryAction.java index 2f5cc8d8..c48cadb7 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIQueryAction.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIQueryAction.java @@ -28,9 +28,9 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryOutput; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; -import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -66,14 +66,11 @@ public class APIQueryAction extends AbstractAPIAction implements QueryInterface int totalCount = 0; while(true) { - try + try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { QQueryFilter filter = queryInput.getFilter(); String paramString = apiActionUtil.buildQueryStringForGet(filter, limit, skip, table.getFields()); - HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); - HttpClient client = httpClientBuilder.build(); - String url = apiActionUtil.buildTableUrl(table) + paramString; LOG.info("API URL: " + url); @@ -86,44 +83,45 @@ public class APIQueryAction extends AbstractAPIAction implements QueryInterface apiActionUtil.setupContentTypeInRequest(request); apiActionUtil.setupAdditionalHeaders(request); - HttpResponse response = client.execute(request); - - int count = apiActionUtil.processGetResponse(table, response, queryOutput); - totalCount += count; - - ///////////////////////////////////////////////////////////////////////// - // if we've fetched at least as many as the original limit, then break // - ///////////////////////////////////////////////////////////////////////// - if(originalLimit != null && totalCount >= originalLimit) + try(CloseableHttpResponse response = httpClient.execute(request)) { - return (queryOutput); - } + int count = apiActionUtil.processGetResponse(table, response, queryOutput); + totalCount += count; - //////////////////////////////////////////////////////////////////////////////////// - // if we got back less than a full page this time, then we must be done, so break // - //////////////////////////////////////////////////////////////////////////////////// - if(count == 0 || (limit != null && count < limit)) - { - return (queryOutput); - } + ///////////////////////////////////////////////////////////////////////// + // if we've fetched at least as many as the original limit, then break // + ///////////////////////////////////////////////////////////////////////// + if(originalLimit != null && totalCount >= originalLimit) + { + return (queryOutput); + } - /////////////////////////////////////////////////////////////////// - // if there's an async callback that says we're cancelled, break // - /////////////////////////////////////////////////////////////////// - if(queryInput.getAsyncJobCallback().wasCancelRequested()) - { - LOG.info("Breaking query job, as requested."); - return (queryOutput); - } + //////////////////////////////////////////////////////////////////////////////////// + // if we got back less than a full page this time, then we must be done, so break // + //////////////////////////////////////////////////////////////////////////////////// + if(count == 0 || (limit != null && count < limit)) + { + return (queryOutput); + } - //////////////////////////////////////////////////////////////////////////// - // else, increment the skip by the count we just got, and query for more. // - //////////////////////////////////////////////////////////////////////////// - if(skip == null) - { - skip = 0; + /////////////////////////////////////////////////////////////////// + // if there's an async callback that says we're cancelled, break // + /////////////////////////////////////////////////////////////////// + if(queryInput.getAsyncJobCallback().wasCancelRequested()) + { + LOG.info("Breaking query job, as requested."); + return (queryOutput); + } + + //////////////////////////////////////////////////////////////////////////// + // else, increment the skip by the count we just got, and query for more. // + //////////////////////////////////////////////////////////////////////////// + if(skip == null) + { + skip = 0; + } + skip += count; } - skip += count; } catch(Exception e) { diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIUpdateAction.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIUpdateAction.java index 5593c612..a74bd5a9 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIUpdateAction.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/APIUpdateAction.java @@ -34,13 +34,11 @@ import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.SleepUtils; import com.kingsrook.qqq.backend.module.api.exceptions.RateLimitException; -import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; -import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -74,17 +72,14 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac QTableMetaData table = updateInput.getTable(); preAction(updateInput); - HttpClientConnectionManager connectionManager = null; - try + try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { - connectionManager = new PoolingHttpClientConnectionManager(); - /////////////////////////////////////////////////////////////// // make post requests for groups of orders that need updated // /////////////////////////////////////////////////////////////// for(List recordList : CollectionUtils.getPages(updateInput.getRecords(), 20)) { - processRecords(table, connectionManager, recordList); + processRecords(table, httpClient, recordList); for(QRecord qRecord : recordList) { updateOutput.addRecord(qRecord); @@ -102,14 +97,6 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac LOG.warn("Error in API Insert for [" + table.getName() + "]", e); throw new QException("Error executing update: " + e.getMessage(), e); } - finally - { - if(connectionManager != null) - { - connectionManager.shutdown(); - } - } - } @@ -117,7 +104,7 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac /******************************************************************************* ** *******************************************************************************/ - private void processRecords(QTableMetaData table, HttpClientConnectionManager connectionManager, List recordList) throws QException + private void processRecords(QTableMetaData table, CloseableHttpClient httpClient, List recordList) throws QException { int sleepMillis = apiActionUtil.getInitialRateLimitBackoffMillis(); int rateLimitsCaught = 0; @@ -125,7 +112,7 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac { try { - doPost(table, connectionManager, recordList); + doPost(table, httpClient, recordList); return; } catch(RateLimitException rle) @@ -149,9 +136,9 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac /******************************************************************************* ** *******************************************************************************/ - private void doPost(QTableMetaData table, HttpClientConnectionManager connectionManager, List recordList) throws RateLimitException, QException + private void doPost(QTableMetaData table, CloseableHttpClient httpClient, List recordList) throws RateLimitException, QException { - try(CloseableHttpClient client = HttpClients.custom().setConnectionManager(connectionManager).build()) + try { String url = apiActionUtil.buildTableUrl(table); HttpPost request = new HttpPost(url); @@ -161,28 +148,30 @@ public class APIUpdateAction extends AbstractAPIAction implements UpdateInterfac request.setEntity(apiActionUtil.recordsToEntity(table, recordList)); - HttpResponse response = client.execute(request); - int statusCode = response.getStatusLine().getStatusCode(); - String responseString = EntityUtils.toString(response.getEntity()); - if(statusCode == HttpStatus.SC_TOO_MANY_REQUESTS) + try(CloseableHttpResponse response = httpClient.execute(request)) { - throw (new RateLimitException(responseString)); - } - if(statusCode != HttpStatus.SC_MULTI_STATUS && statusCode != HttpStatus.SC_OK) - { - String errorMessage = "Did not receive response status code of 200 or 207: " + responseString; - LOG.warn(errorMessage); - throw (new QException(errorMessage)); - } - if(statusCode == HttpStatus.SC_MULTI_STATUS) - { - JSONObject responseJSON = new JSONObject(responseString).getJSONObject("response"); - if(!responseJSON.optString("status").contains("200 OK")) + int statusCode = response.getStatusLine().getStatusCode(); + String responseString = EntityUtils.toString(response.getEntity()); + if(statusCode == HttpStatus.SC_TOO_MANY_REQUESTS) { - String errorMessage = "Did not receive ok status response: " + responseJSON.optString("description"); + throw (new RateLimitException(responseString)); + } + if(statusCode != HttpStatus.SC_MULTI_STATUS && statusCode != HttpStatus.SC_OK) + { + String errorMessage = "Did not receive response status code of 200 or 207: " + responseString; LOG.warn(errorMessage); throw (new QException(errorMessage)); } + if(statusCode == HttpStatus.SC_MULTI_STATUS) + { + JSONObject responseJSON = new JSONObject(responseString).getJSONObject("response"); + if(!responseJSON.optString("status").contains("200 OK")) + { + String errorMessage = "Did not receive ok status response: " + responseJSON.optString("description"); + LOG.warn(errorMessage); + throw (new QException(errorMessage)); + } + } } } catch(RateLimitException | QException e) diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/EasyPostApiTest.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/EasyPostApiTest.java index 81e4427a..d5f3ede4 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/EasyPostApiTest.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/EasyPostApiTest.java @@ -64,7 +64,7 @@ public class EasyPostApiTest { QRecord record = new QRecord() .withValue("__ignoreMe", "123") - .withValue("carrierCode", "USPS") + .withValue("carrier", "USPS") .withValue("trackingNo", "EZ4000000004"); InsertInput insertInput = new InsertInput(TestUtils.defineInstance()); @@ -79,6 +79,30 @@ public class EasyPostApiTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testPostMultiple() throws QException + { + QRecord record1 = new QRecord().withValue("carrier", "USPS").withValue("trackingNo", "EZ1000000001"); + QRecord record2 = new QRecord().withValue("carrier", "USPS").withValue("trackingNo", "EZ2000000002"); + + InsertInput insertInput = new InsertInput(TestUtils.defineInstance()); + insertInput.setSession(new QSession()); + insertInput.setTableName("easypostTracker"); + insertInput.setRecords(List.of(record1, record2)); + InsertOutput insertOutput = new InsertAction().execute(insertInput); + + QRecord outputRecord0 = insertOutput.getRecords().get(0); + assertNotNull(outputRecord0.getValue("id"), "Should get a tracker id"); + + QRecord outputRecord1 = insertOutput.getRecords().get(1); + assertNotNull(outputRecord1.getValue("id"), "Should get a tracker id"); + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -109,7 +133,7 @@ public class EasyPostApiTest ((APIBackendMetaData) backend).setApiKey("not-valid"); QRecord record = new QRecord() - .withValue("carrierCode", "USPS") + .withValue("carrier", "USPS") .withValue("trackingNo", "EZ1000000001"); InsertInput insertInput = new InsertInput(instance); @@ -132,7 +156,7 @@ public class EasyPostApiTest void testPostTrackerError() throws QException { QRecord record = new QRecord() - .withValue("carrierCode", "USPS") + .withValue("carrier", "USPS") .withValue("trackingNo", "Not-Valid-Tracking-No"); InsertInput insertInput = new InsertInput(TestUtils.defineInstance()); diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/TestUtils.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/TestUtils.java index ba76bae2..5388297f 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/TestUtils.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/TestUtils.java @@ -102,7 +102,7 @@ public class TestUtils .withBackendName("easypost") .withField(new QFieldMetaData("id", QFieldType.STRING)) .withField(new QFieldMetaData("trackingNo", QFieldType.STRING).withBackendName("tracking_code")) - .withField(new QFieldMetaData("carrierCode", QFieldType.STRING).withBackendName("carrier")) + .withField(new QFieldMetaData("carrier", QFieldType.STRING).withBackendName("carrier")) .withPrimaryKeyField("id") .withBackendDetails(new APITableBackendDetails() .withTablePath("trackers")