From 489f12996de4d4a8c85527aed43a1853ad984e6b Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 25 May 2023 11:34:50 -0500 Subject: [PATCH 1/7] updated to allow 'trying again' when server side 500 error occur in makeRequest() --- .../module/api/actions/BaseAPIActionUtil.java | 60 +++++++++++++++-- .../RetryableServerErrorException.java | 42 ++++++++++++ .../api/actions/BaseAPIActionUtilTest.java | 67 +++++++++++++++---- 3 files changed, 151 insertions(+), 18 deletions(-) create mode 100644 qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java index 7b8769e7..7a3c46af 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java @@ -63,6 +63,7 @@ import com.kingsrook.qqq.backend.core.utils.ValueUtils; import com.kingsrook.qqq.backend.module.api.exceptions.OAuthCredentialsException; import com.kingsrook.qqq.backend.module.api.exceptions.OAuthExpiredTokenException; import com.kingsrook.qqq.backend.module.api.exceptions.RateLimitException; +import com.kingsrook.qqq.backend.module.api.exceptions.RetryableServerErrorException; import com.kingsrook.qqq.backend.module.api.model.AuthorizationType; import com.kingsrook.qqq.backend.module.api.model.OutboundAPILog; import com.kingsrook.qqq.backend.module.api.model.metadata.APIBackendMetaData; @@ -878,8 +879,10 @@ public class BaseAPIActionUtil *******************************************************************************/ public QHttpResponse makeRequest(QTableMetaData table, HttpRequestBase request) throws QException { - int sleepMillis = getInitialRateLimitBackoffMillis(); + int rateLimitSleepMillis = getInitialRateLimitBackoffMillis(); + int serverErrorsSleepMillis = getInitialServerErrorBackoffMillis(); int rateLimitsCaught = 0; + int serverErrorsCaught = 0; boolean caughtAnOAuthExpiredToken = false; while(true) @@ -913,7 +916,11 @@ public class BaseAPIActionUtil { throw (new RateLimitException(qResponse.getContent())); } - if(statusCode >= 400) + else if(shouldBeRetryableServerErrorException(qResponse)) + { + throw (new RetryableServerErrorException(qResponse.getContent())); + } + else if(statusCode >= 400) { handleResponseError(table, request, qResponse); } @@ -950,9 +957,22 @@ public class BaseAPIActionUtil throw (new QException(rle)); } - LOG.info("Caught RateLimitException", logPair("rateLimitsCaught", rateLimitsCaught), logPair("uri", request.getURI()), logPair("table", table.getName()), logPair("sleeping", sleepMillis)); - SleepUtils.sleep(sleepMillis, TimeUnit.MILLISECONDS); - sleepMillis *= 2; + LOG.info("Caught RateLimitException", logPair("rateLimitsCaught", rateLimitsCaught), logPair("uri", request.getURI()), logPair("table", table.getName()), logPair("sleeping", rateLimitSleepMillis)); + SleepUtils.sleep(rateLimitSleepMillis, TimeUnit.MILLISECONDS); + rateLimitSleepMillis *= 2; + } + catch(RetryableServerErrorException see) + { + serverErrorsCaught++; + if(serverErrorsCaught > getMaxAllowedServerErrors()) + { + LOG.error("Giving up " + request.getMethod() + " to [" + table.getName() + "] after too many server-side errors (" + getMaxAllowedServerErrors() + ")"); + throw (new QException(see)); + } + + LOG.info("Caught Server-side error during API request", logPair("serverErrorsCaught", serverErrorsCaught), logPair("uri", request.getURI()), logPair("table", table.getName()), logPair("sleeping", serverErrorsSleepMillis)); + SleepUtils.sleep(serverErrorsSleepMillis, TimeUnit.MILLISECONDS); + serverErrorsSleepMillis *= 2; } catch(QException qe) { @@ -972,6 +992,16 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** + *******************************************************************************/ + private boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) + { + return (qResponse.getStatusCode() != null && qResponse.getStatusCode() >= 500); + } + + + /******************************************************************************* ** one-line method, factored out so mock/tests can override *******************************************************************************/ @@ -1141,6 +1171,16 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** + *******************************************************************************/ + protected int getInitialServerErrorBackoffMillis() + { + return (500); + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -1151,6 +1191,16 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** + *******************************************************************************/ + protected int getMaxAllowedServerErrors() + { + return (3); + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java new file mode 100644 index 00000000..33f79016 --- /dev/null +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java @@ -0,0 +1,42 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2022. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.module.api.exceptions; + + +import com.kingsrook.qqq.backend.core.exceptions.QException; + + +/******************************************************************************* + ** + *******************************************************************************/ +public class RetryableServerErrorException extends QException +{ + + /******************************************************************************* + ** + *******************************************************************************/ + public RetryableServerErrorException(String message) + { + super(message); + } + +} diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java index 3930d5b6..a2f195b1 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java @@ -124,9 +124,16 @@ class BaseAPIActionUtilTest extends BaseTest // avoid the fully mocked makeRequest // //////////////////////////////////////// mockApiUtilsHelper.setUseMock(false); - mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" - {"error": "Server error"} - """)); + + ////////////////////////// + // set to retry 3 times // + ////////////////////////// + for(int i = 0; i < 4; i++) + { + mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" + {"error": "Server error"} + """)); + } CountInput countInput = new CountInput(); countInput.setTableName(TestUtils.MOCK_TABLE_NAME); @@ -290,9 +297,16 @@ class BaseAPIActionUtilTest extends BaseTest // avoid the fully mocked makeRequest // //////////////////////////////////////// mockApiUtilsHelper.setUseMock(false); - mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" - {"error": "Server error"} - """)); + + ////////////////////////// + // set to retry 3 times // + ////////////////////////// + for(int i = 0; i < 4; i++) + { + mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" + {"error": "Server error"} + """)); + } QueryInput queryInput = new QueryInput(); queryInput.setTableName(TestUtils.MOCK_TABLE_NAME); @@ -344,9 +358,16 @@ class BaseAPIActionUtilTest extends BaseTest // avoid the fully mocked makeRequest // //////////////////////////////////////// mockApiUtilsHelper.setUseMock(false); - mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" - {"error": "Server error"} - """)); + + ////////////////////////// + // set to retry 3 times // + ////////////////////////// + for(int i = 0; i < 4; i++) + { + mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" + {"error": "Server error"} + """)); + } InsertInput insertInput = new InsertInput(); insertInput.setRecords(List.of(new QRecord().withValue("name", "Milhouse"))); @@ -411,9 +432,13 @@ class BaseAPIActionUtilTest extends BaseTest // avoid the fully mocked makeRequest // //////////////////////////////////////// mockApiUtilsHelper.setUseMock(false); - mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" - {"error": "Server error"} - """)); + + for(int i = 0; i < 4; i++) + { + mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(500).withContent(""" + {"error": "Server error"} + """)); + } UpdateInput updateInput = new UpdateInput(); updateInput.setRecords(List.of(new QRecord().withValue("name", "Milhouse"))); @@ -682,4 +707,20 @@ class BaseAPIActionUtilTest extends BaseTest return (new GetAction().execute(getInput)); } -} \ No newline at end of file + + + /******************************************************************************* + ** subclass of base api action utils that can be used to test overriding methods + *******************************************************************************/ + private class BaseAPIActionUtilSubclass extends BaseAPIActionUtil + { + /******************************************************************************* + ** + *******************************************************************************/ + private boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) + { + return (false); + } + + } +} From 343f3fe01ab29b36e63c887e4ec10766da75d2c5 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 30 May 2023 11:19:22 -0500 Subject: [PATCH 2/7] Update to support both JSON and multipart form bodies for create and update. --- .../javalin/QJavalinImplementation.java | 120 ++++++++++++------ .../javalin/QJavalinImplementationTest.java | 82 +++++++++++- 2 files changed, 161 insertions(+), 41 deletions(-) diff --git a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java index be89714c..7fb757fa 100644 --- a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java +++ b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementation.java @@ -611,38 +611,17 @@ public class QJavalinImplementation updateInput.setTableName(table); PermissionsHelper.checkTablePermissionThrowing(updateInput, TablePermissionSubType.EDIT); + QTableMetaData tableMetaData = qInstance.getTable(table); + + QJavalinAccessLogger.logStart("update", logPair("table", table), logPair("primaryKey", primaryKey)); List recordList = new ArrayList<>(); QRecord record = new QRecord(); record.setTableName(table); recordList.add(record); - Map map = context.bodyAsClass(Map.class); - for(Map.Entry entry : map.entrySet()) - { - String fieldName = ValueUtils.getValueAsString(entry.getKey()); - Object value = entry.getValue(); - - if(StringUtils.hasContent(String.valueOf(value))) - { - record.setValue(fieldName, (Serializable) value); - } - else if("".equals(value)) - { - /////////////////////////////////////////////////////////////////////////////////////////////////// - // if frontend sent us an empty string - put a null in the record's value map. // - // this could potentially be changed to be type-specific (e.g., store an empty-string for STRING // - // fields, but null for INTEGER, etc) - but, who really wants empty-string in database anyway? // - /////////////////////////////////////////////////////////////////////////////////////////////////// - record.setValue(fieldName, null); - } - } - - QTableMetaData tableMetaData = qInstance.getTable(table); record.setValue(tableMetaData.getPrimaryKeyField(), primaryKey); - - QJavalinAccessLogger.logStart("update", logPair("table", table), logPair("primaryKey", primaryKey)); - + setRecordValuesForInsertOrUpdate(context, tableMetaData, record); updateInput.setRecords(recordList); UpdateAction updateAction = new UpdateAction(); @@ -660,6 +639,80 @@ public class QJavalinImplementation + /******************************************************************************* + ** + *******************************************************************************/ + private static void setRecordValuesForInsertOrUpdate(Context context, QTableMetaData tableMetaData, QRecord record) throws IOException + { + String contentType = Objects.requireNonNullElse(context.header("content-type"), ""); + boolean isContentTypeJson = contentType.toLowerCase().contains("json"); + + try + { + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // if the caller said they've sent JSON, or if they didn't supply a content-type, then try to read the body // + // as JSON. if it throws, we'll continue by trying to read form params, but if it succeeds, we'll return. // + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + if(isContentTypeJson || !StringUtils.hasContent(contentType)) + { + Map map = context.bodyAsClass(Map.class); + for(Map.Entry entry : map.entrySet()) + { + String fieldName = ValueUtils.getValueAsString(entry.getKey()); + Object value = entry.getValue(); + + if(StringUtils.hasContent(String.valueOf(value))) + { + record.setValue(fieldName, (Serializable) value); + } + else if("".equals(value)) + { + /////////////////////////////////////////////////////////////////////////////////////////////////// + // if frontend sent us an empty string - put a null in the record's value map. // + // this could potentially be changed to be type-specific (e.g., store an empty-string for STRING // + // fields, but null for INTEGER, etc) - but, who really wants empty-string in database anyway? // + /////////////////////////////////////////////////////////////////////////////////////////////////// + record.setValue(fieldName, null); + } + } + + return; + } + } + catch(Exception e) + { + LOG.info("Error trying to read body as map", e, logPair("contentType", contentType)); + } + + ///////////////////////// + // process form params // + ///////////////////////// + for(Map.Entry> formParam : context.formParamMap().entrySet()) + { + String fieldName = formParam.getKey(); + List values = formParam.getValue(); + if(CollectionUtils.nullSafeHasContents(values)) + { + String value = values.get(0); + if(StringUtils.hasContent(value)) + { + record.setValue(fieldName, value); + } + else + { + record.setValue(fieldName, null); + } + } + else + { + // is this ever hit? + record.setValue(fieldName, null); + } + } + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -674,20 +727,13 @@ public class QJavalinImplementation QJavalinAccessLogger.logStart("insert", logPair("table", tableName)); PermissionsHelper.checkTablePermissionThrowing(insertInput, TablePermissionSubType.INSERT); + QTableMetaData tableMetaData = qInstance.getTable(tableName); List recordList = new ArrayList<>(); QRecord record = new QRecord(); record.setTableName(tableName); recordList.add(record); - - Map map = context.bodyAsClass(Map.class); - for(Map.Entry entry : map.entrySet()) - { - if(StringUtils.hasContent(String.valueOf(entry.getValue()))) - { - record.setValue(String.valueOf(entry.getKey()), (Serializable) entry.getValue()); - } - } + setRecordValuesForInsertOrUpdate(context, tableMetaData, record); insertInput.setRecords(recordList); InsertAction insertAction = new InsertAction(); @@ -695,14 +741,14 @@ public class QJavalinImplementation if(CollectionUtils.nullSafeHasContents(insertOutput.getRecords().get(0).getErrors())) { - throw (new QUserFacingException("Error inserting " + qInstance.getTable(tableName).getLabel() + ": " + insertOutput.getRecords().get(0).getErrors().get(0))); + throw (new QUserFacingException("Error inserting " + tableMetaData.getLabel() + ": " + insertOutput.getRecords().get(0).getErrors().get(0))); } if(CollectionUtils.nullSafeHasContents(insertOutput.getRecords().get(0).getWarnings())) { - throw (new QUserFacingException("Warning inserting " + qInstance.getTable(tableName).getLabel() + ": " + insertOutput.getRecords().get(0).getWarnings().get(0))); + throw (new QUserFacingException("Warning inserting " + tableMetaData.getLabel() + ": " + insertOutput.getRecords().get(0).getWarnings().get(0))); } - QJavalinAccessLogger.logEndSuccess(logPair("primaryKey", () -> (insertOutput.getRecords().get(0).getValue(qInstance.getTable(tableName).getPrimaryKeyField())))); + QJavalinAccessLogger.logEndSuccess(logPair("primaryKey", () -> (insertOutput.getRecords().get(0).getValue(tableMetaData.getPrimaryKeyField())))); context.result(JsonUtils.toJson(insertOutput)); } catch(Exception e) diff --git a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java index 0fa2e777..722cba48 100644 --- a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java +++ b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java @@ -391,11 +391,13 @@ class QJavalinImplementationTest extends QJavalinTestBase /******************************************************************************* - ** test an insert + ** test an insert - posting the data as a JSON object. + ** + ** This was the original supported version, but multipart form was added in May 2023 ** *******************************************************************************/ @Test - public void test_dataInsert() + public void test_dataInsertJson() { Map body = new HashMap<>(); body.put("firstName", "Bobby"); @@ -425,11 +427,45 @@ class QJavalinImplementationTest extends QJavalinTestBase /******************************************************************************* - ** test an update + ** test an insert - posting a multipart form. ** *******************************************************************************/ @Test - public void test_dataUpdate() + public void test_dataInsertMultipartForm() + { + HttpResponse response = Unirest.post(BASE_URL + "/data/person") + .header("Content-Type", "application/json") + .multiPartContent() + .field("firstName", "Bobby") + .field("lastName", "Hull") + .field("email", "bobby@hull.com") + .asString(); + + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertTrue(jsonObject.has("records")); + JSONArray records = jsonObject.getJSONArray("records"); + assertEquals(1, records.length()); + JSONObject record0 = records.getJSONObject(0); + assertTrue(record0.has("values")); + assertEquals("person", record0.getString("tableName")); + JSONObject values0 = record0.getJSONObject("values"); + assertTrue(values0.has("firstName")); + assertEquals("Bobby", values0.getString("firstName")); + assertTrue(values0.has("id")); + assertEquals(7, values0.getInt("id")); + } + + + + /******************************************************************************* + ** test an update - posting the data as a JSON object. + ** + ** This was the original supported version, but multipart form was added in May 2023 + ** + *******************************************************************************/ + @Test + public void test_dataUpdateJson() { Map body = new HashMap<>(); body.put("firstName", "Free"); @@ -465,6 +501,44 @@ class QJavalinImplementationTest extends QJavalinTestBase + /******************************************************************************* + ** test an update - posting the data as a multipart form + ** + *******************************************************************************/ + @Test + public void test_dataUpdateMultipartForm() + { + HttpResponse response = Unirest.patch(BASE_URL + "/data/person/4") + .multiPartContent() + .field("firstName", "Free") + .field("birthDate", "") + .asString(); + + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertTrue(jsonObject.has("records")); + JSONArray records = jsonObject.getJSONArray("records"); + assertEquals(1, records.length()); + JSONObject record0 = records.getJSONObject(0); + assertTrue(record0.has("values")); + assertEquals("person", record0.getString("tableName")); + JSONObject values0 = record0.getJSONObject("values"); + assertEquals(4, values0.getInt("id")); + assertEquals("Free", values0.getString("firstName")); + + /////////////////////////////////////////////////////////////////// + // re-GET the record, and validate that birthDate was nulled out // + /////////////////////////////////////////////////////////////////// + response = Unirest.get(BASE_URL + "/data/person/4").asString(); + assertEquals(200, response.getStatus()); + jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertTrue(jsonObject.has("values")); + JSONObject values = jsonObject.getJSONObject("values"); + assertFalse(values.has("birthDate")); + } + + + /******************************************************************************* ** test a delete ** From a5387ff9dbde1cecc49ddb7941ae9810b2aea0bd Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 30 May 2023 14:04:30 -0500 Subject: [PATCH 3/7] added retryable exception and method that can be overridden in base classes to allow retrying in different circumstances, added SC_GATEWAY_TIMEOUT as a logger.info --- .../module/api/actions/BaseAPIActionUtil.java | 8 ++-- .../RetryableServerErrorException.java | 37 ++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java index 7a3c46af..14f86167 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java @@ -514,7 +514,7 @@ public class BaseAPIActionUtil { return; } - else if(statusCode == HttpStatus.SC_BAD_GATEWAY) + else if(statusCode == HttpStatus.SC_BAD_GATEWAY || statusCode == HttpStatus.SC_GATEWAY_TIMEOUT) { LOG.info("HTTP " + request.getMethod() + " failed", logPair("table", table.getName()), logPair("statusCode", statusCode), logPair("responseContent", StringUtils.safeTruncate(resultString, 1024, "..."))); didLog = true; @@ -918,7 +918,7 @@ public class BaseAPIActionUtil } else if(shouldBeRetryableServerErrorException(qResponse)) { - throw (new RetryableServerErrorException(qResponse.getContent())); + throw (new RetryableServerErrorException(statusCode, qResponse.getContent())); } else if(statusCode >= 400) { @@ -970,7 +970,7 @@ public class BaseAPIActionUtil throw (new QException(see)); } - LOG.info("Caught Server-side error during API request", logPair("serverErrorsCaught", serverErrorsCaught), logPair("uri", request.getURI()), logPair("table", table.getName()), logPair("sleeping", serverErrorsSleepMillis)); + LOG.info("Caught Server-side error during API request", logPair("serverErrorsCaught", serverErrorsCaught), logPair("uri", request.getURI()), logPair("code", see.getCode()), logPair("table", table.getName()), logPair("sleeping", serverErrorsSleepMillis)); SleepUtils.sleep(serverErrorsSleepMillis, TimeUnit.MILLISECONDS); serverErrorsSleepMillis *= 2; } @@ -995,7 +995,7 @@ public class BaseAPIActionUtil /******************************************************************************* ** *******************************************************************************/ - private boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) + protected boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) { return (qResponse.getStatusCode() != null && qResponse.getStatusCode() >= 500); } diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java index 33f79016..5e469a80 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/exceptions/RetryableServerErrorException.java @@ -30,13 +30,48 @@ import com.kingsrook.qqq.backend.core.exceptions.QException; *******************************************************************************/ public class RetryableServerErrorException extends QException { + private Integer code; + + /******************************************************************************* ** *******************************************************************************/ - public RetryableServerErrorException(String message) + public RetryableServerErrorException(Integer code, String message) { super(message); + this.code = code; + } + + + + /******************************************************************************* + ** Getter for code + *******************************************************************************/ + public Integer getCode() + { + return (this.code); + } + + + + /******************************************************************************* + ** Setter for code + *******************************************************************************/ + public void setCode(Integer code) + { + this.code = code; + } + + + + /******************************************************************************* + ** Fluent setter for code + *******************************************************************************/ + public RetryableServerErrorException withCode(Integer code) + { + this.code = code; + return (this); } } From ca1cc853fc325ff67aca43fcfa359c5ffcda498e Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 30 May 2023 14:55:56 -0500 Subject: [PATCH 4/7] fixed access on subclass method in test --- .../qqq/backend/module/api/actions/BaseAPIActionUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java index a2f195b1..30209807 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java @@ -717,7 +717,7 @@ class BaseAPIActionUtilTest extends BaseTest /******************************************************************************* ** *******************************************************************************/ - private boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) + protected boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) { return (false); } From 6455171ceed75a656ab719c94dbe79f9bd37e8d3 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 30 May 2023 14:57:34 -0500 Subject: [PATCH 5/7] removed unused subclass --- .../api/actions/BaseAPIActionUtilTest.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java index 30209807..7ec5dc3d 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java @@ -707,20 +707,4 @@ class BaseAPIActionUtilTest extends BaseTest return (new GetAction().execute(getInput)); } - - - /******************************************************************************* - ** subclass of base api action utils that can be used to test overriding methods - *******************************************************************************/ - private class BaseAPIActionUtilSubclass extends BaseAPIActionUtil - { - /******************************************************************************* - ** - *******************************************************************************/ - protected boolean shouldBeRetryableServerErrorException(QHttpResponse qResponse) - { - return (false); - } - - } } From f5516c8122f5c23c0ef739de025daaba6832b7ca Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Jun 2023 15:02:32 -0500 Subject: [PATCH 6/7] Make column stats treat "" and null the same (as a non-value or blank) --- .../memory/MemoryAggregateAction.java | 53 ++++ .../memory/MemoryBackendModule.java | 12 + .../memory/MemoryRecordStore.java | 275 ++++++++++++++++++ .../columnstats/ColumnStatsStep.java | 159 ++++++++-- .../memory/MemoryBackendModuleTest.java | 145 +++++++++ .../columnstats/ColumnStatsStepTest.java | 61 ++++ 6 files changed, 680 insertions(+), 25 deletions(-) create mode 100644 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryAggregateAction.java create mode 100644 qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStepTest.java diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryAggregateAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryAggregateAction.java new file mode 100644 index 00000000..bfc2bba0 --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryAggregateAction.java @@ -0,0 +1,53 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2022. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.core.modules.backend.implementations.memory; + + +import com.kingsrook.qqq.backend.core.actions.interfaces.AggregateInterface; +import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOutput; + + +/******************************************************************************* + ** In-memory version of aggregate action. + ** + *******************************************************************************/ +public class MemoryAggregateAction implements AggregateInterface +{ + + /******************************************************************************* + ** + *******************************************************************************/ + public AggregateOutput execute(AggregateInput aggregateInput) throws QException + { + try + { + return (MemoryRecordStore.getInstance().aggregate(aggregateInput)); + } + catch(Exception e) + { + throw new QException("Error executing aggregate", e); + } + } + +} diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModule.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModule.java index cb423dc5..4d6a93cb 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModule.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModule.java @@ -22,6 +22,7 @@ package com.kingsrook.qqq.backend.core.modules.backend.implementations.memory; +import com.kingsrook.qqq.backend.core.actions.interfaces.AggregateInterface; import com.kingsrook.qqq.backend.core.actions.interfaces.CountInterface; import com.kingsrook.qqq.backend.core.actions.interfaces.DeleteInterface; import com.kingsrook.qqq.backend.core.actions.interfaces.InsertInterface; @@ -74,6 +75,17 @@ public class MemoryBackendModule implements QBackendModuleInterface + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public AggregateInterface getAggregateInterface() + { + return new MemoryAggregateAction(); + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryRecordStore.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryRecordStore.java index b25cac78..eeef3387 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryRecordStore.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryRecordStore.java @@ -23,22 +23,36 @@ package com.kingsrook.qqq.backend.core.modules.backend.implementations.memory; import java.io.Serializable; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; import com.kingsrook.qqq.backend.core.actions.tables.helpers.ValidateRecordSecurityLockHelper; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.actions.AbstractActionInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.Aggregate; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOperator; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOutput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateResult; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.GroupBy; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByAggregate; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByGroupBy; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteInput; import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.JoinsContext; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryJoin; import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput; @@ -525,4 +539,265 @@ public class MemoryRecordStore return (actionInputs); } + + + /******************************************************************************* + ** + *******************************************************************************/ + public AggregateOutput aggregate(AggregateInput aggregateInput) throws QException + { + ////////////////////// + // first do a query // + ////////////////////// + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(aggregateInput.getTableName()); + queryInput.setFilter(aggregateInput.getFilter()); + queryInput.setQueryJoins(aggregateInput.getQueryJoins()); + List queryResult = query(queryInput); + + List results = new ArrayList<>(); + List groupBys = CollectionUtils.nonNullList(aggregateInput.getGroupBys()); + List aggregates = CollectionUtils.nonNullList(aggregateInput.getAggregates()); + + ///////////////////// + // do the group-by // + ///////////////////// + ListingHash, QRecord> bins = new ListingHash<>(); + for(QRecord record : queryResult) + { + List groupByValues = new ArrayList<>(groupBys.size()); + for(GroupBy groupBy : groupBys) + { + Serializable groupByValue = record.getValue(groupBy.getFieldName()); + if(groupBy.getType() != null) + { + groupByValue = ValueUtils.getValueAsFieldType(groupBy.getType(), groupByValue); + } + groupByValues.add(groupByValue); + } + + bins.add(groupByValues, record); + } + + //////////////////////// + // do the aggregating // + //////////////////////// + for(Map.Entry, List> entry : bins.entrySet()) + { + List groupByValueList = entry.getKey(); + List records = entry.getValue(); + + AggregateResult aggregateResult = new AggregateResult(); + results.add(aggregateResult); + + //////////////////////////////////////////// + // set the group-by values in this result // + //////////////////////////////////////////// + Map groupByValues = new HashMap<>(); + aggregateResult.setGroupByValues(groupByValues); + for(int i = 0; i < groupBys.size(); i++) + { + GroupBy groupBy = groupBys.get(i); + Serializable value = groupByValueList.get(i); + groupByValues.put(groupBy, value); + } + + //////////////////////////// + // compute the aggregates // + //////////////////////////// + Map aggregateValues = new HashMap<>(); + aggregateResult.setAggregateValues(aggregateValues); + + for(Aggregate aggregate : aggregates) + { + Serializable aggregateValue = computeAggregate(records, aggregate, aggregateInput.getTable()); + + aggregateValues.put(aggregate, aggregateValue); + } + } + + ///////////////////// + // sort the result // + ///////////////////// + if(aggregateInput.getFilter() != null && CollectionUtils.nullSafeHasContents(aggregateInput.getFilter().getOrderBys())) + { + Comparator comparator = null; + Comparator serializableComparator = (Serializable a, Serializable b) -> + { + if(a == null && b == null) + { + return (0); + } + else if(a == null) + { + return (1); + } + else if(b == null) + { + return (-1); + } + return ((Comparable) a).compareTo(b); + }; + + //////////////////////////////////////////////// + // build a comparator out of all the orderBys // + //////////////////////////////////////////////// + for(QFilterOrderBy orderBy : aggregateInput.getFilter().getOrderBys()) + { + Function keyExtractor = aggregateResult -> + { + if(orderBy instanceof QFilterOrderByGroupBy orderByGroupBy) + { + return aggregateResult.getGroupByValue(orderByGroupBy.getGroupBy()); + } + else if(orderBy instanceof QFilterOrderByAggregate orderByAggregate) + { + return aggregateResult.getAggregateValue(orderByAggregate.getAggregate()); + } + else + { + throw (new IllegalStateException("Unexpected orderBy [" + orderBy + "] in aggregate")); + } + }; + + if(comparator == null) + { + comparator = Comparator.comparing(keyExtractor, serializableComparator); + } + else + { + comparator = comparator.thenComparing(keyExtractor, serializableComparator); + } + + if(!orderBy.getIsAscending()) + { + comparator = comparator.reversed(); + } + } + + /////////////////////////////////////// + // sort the list with the comparator // + /////////////////////////////////////// + results.sort(comparator); + } + + AggregateOutput aggregateOutput = new AggregateOutput(); + aggregateOutput.setResults(results); + return (aggregateOutput); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @SuppressWarnings("checkstyle:indentation") + private static Serializable computeAggregate(List records, Aggregate aggregate, QTableMetaData table) + { + String fieldName = aggregate.getFieldName(); + AggregateOperator operator = aggregate.getOperator(); + QFieldType fieldType; + if(aggregate.getFieldType() == null) + { + // todo - joins probably? + QFieldMetaData field = table.getField(fieldName); + if(field.getType().equals(QFieldType.INTEGER) && (operator.equals(AggregateOperator.AVG))) + { + fieldType = QFieldType.DECIMAL; + } + else if(operator.equals(AggregateOperator.COUNT) || operator.equals(AggregateOperator.COUNT_DISTINCT)) + { + fieldType = QFieldType.INTEGER; + } + else + { + fieldType = field.getType(); + } + } + else + { + fieldType = aggregate.getFieldType(); + } + + Serializable aggregateValue = switch(operator) + { + case COUNT -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .count(); + + case COUNT_DISTINCT -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .map(r -> r.getValue(fieldName)) + .collect(Collectors.toSet()) + .size(); + + case SUM -> switch(fieldType) + { + case INTEGER -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .mapToInt(r -> r.getValueInteger(fieldName)) + .sum(); + case DECIMAL -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .map(r -> r.getValueBigDecimal(fieldName)) + .reduce(BigDecimal.ZERO, BigDecimal::add); + default -> throw (new IllegalArgumentException("Cannot perform " + operator + " aggregate on " + fieldType + " field.")); + }; + + case MIN -> switch(fieldType) + { + case INTEGER -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .mapToInt(r -> r.getValueInteger(fieldName)) + .min() + .stream().boxed().findFirst().orElse(null); + case DECIMAL, STRING, DATE, DATE_TIME -> + { + Optional serializable = records.stream() + .filter(r -> r.getValue(fieldName) != null) + .map(r -> ((Comparable) ValueUtils.getValueAsFieldType(fieldType, r.getValue(fieldName)))) + .min(Comparator.naturalOrder()) + .map(c -> (Serializable) c); + yield serializable.orElse(null); + } + default -> throw (new IllegalArgumentException("Cannot perform " + operator + " aggregate on " + fieldType + " field.")); + }; + + case MAX -> switch(fieldType) + { + case INTEGER -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .mapToInt(r -> r.getValueInteger(fieldName)) + .max() + .stream().boxed().findFirst().orElse(null); + case DECIMAL, STRING, DATE, DATE_TIME -> + { + Optional serializable = records.stream() + .filter(r -> r.getValue(fieldName) != null) + .map(r -> ((Comparable) ValueUtils.getValueAsFieldType(fieldType, r.getValue(fieldName)))) + .max(Comparator.naturalOrder()) + .map(c -> (Serializable) c); + yield serializable.orElse(null); + } + default -> throw (new IllegalArgumentException("Cannot perform " + operator + " aggregate on " + fieldType + " field.")); + }; + + case AVG -> switch(fieldType) + { + case INTEGER -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .mapToInt(r -> r.getValueInteger(fieldName)) + .average() + .stream().boxed().findFirst().orElse(null); + case DECIMAL -> records.stream() + .filter(r -> r.getValue(fieldName) != null) + .mapToDouble(r -> r.getValueBigDecimal(fieldName).doubleValue()) + .average() + .stream().boxed().map(d -> new BigDecimal(d)).findFirst().orElse(null); + default -> throw (new IllegalArgumentException("Cannot perform " + operator + " aggregate on " + fieldType + " field.")); + }; + }; + + return ValueUtils.getValueAsFieldType(fieldType, aggregateValue); + } } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStep.java index dee5e4d1..2bb90e28 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStep.java @@ -24,8 +24,13 @@ package com.kingsrook.qqq.backend.core.processes.implementations.columnstats; import java.io.Serializable; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; +import com.kingsrook.qqq.backend.core.actions.dashboard.widgets.DateTimeGroupBy; import com.kingsrook.qqq.backend.core.actions.permissions.PermissionsHelper; import com.kingsrook.qqq.backend.core.actions.permissions.TablePermissionSubType; import com.kingsrook.qqq.backend.core.actions.processes.BackendStep; @@ -45,7 +50,7 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOu import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateResult; import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.GroupBy; import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByAggregate; -import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByGroupBy; 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.QueryJoin; @@ -59,6 +64,7 @@ import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.JsonUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; import com.kingsrook.qqq.backend.core.utils.ValueUtils; +import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; /******************************************************************************* @@ -140,6 +146,14 @@ public class ColumnStatsStep implements BackendStep Aggregate aggregate = new Aggregate(table.getPrimaryKeyField(), AggregateOperator.COUNT).withFieldType(QFieldType.DECIMAL); GroupBy groupBy = new GroupBy(field.getType(), fieldName); + // todo - something here about "by-date, not time" + if(field.getType().equals(QFieldType.DATE_TIME)) + { + // groupBy = new GroupBy(field.getType(), fieldName, "DATE(%s)"); + String sqlExpression = DateTimeGroupBy.HOUR.getSqlExpression(); + groupBy = new GroupBy(QFieldType.STRING, fieldName, sqlExpression); + } + if(StringUtils.hasContent(orderBy)) { if(orderBy.equalsIgnoreCase("count.asc")) @@ -152,11 +166,11 @@ public class ColumnStatsStep implements BackendStep } else if(orderBy.equalsIgnoreCase(fieldName + ".asc")) { - filter.withOrderBy(new QFilterOrderBy(fieldName, true)); + filter.withOrderBy(new QFilterOrderByGroupBy(groupBy, true)); } else if(orderBy.equalsIgnoreCase(fieldName + ".desc")) { - filter.withOrderBy(new QFilterOrderBy(fieldName, false)); + filter.withOrderBy(new QFilterOrderByGroupBy(groupBy, false)); } else { @@ -168,7 +182,7 @@ public class ColumnStatsStep implements BackendStep // always add order by to break ties. these will be the default too, if input didn't supply one // /////////////////////////////////////////////////////////////////////////////////////////////////// filter.withOrderBy(new QFilterOrderByAggregate(aggregate, false)); - filter.withOrderBy(new QFilterOrderBy(fieldName)); + filter.withOrderBy(new QFilterOrderByGroupBy(groupBy)); Integer limit = 1000; // too big? AggregateInput aggregateInput = new AggregateInput(); @@ -192,6 +206,14 @@ public class ColumnStatsStep implements BackendStep Integer count = ValueUtils.getValueAsInteger(result.getAggregateValue(aggregate)); valueCounts.add(new QRecord().withValue(fieldName, value).withValue("count", count)); } + + ////////////////////////////////////////////////////////////////////////////////////////////////// + // so... our json serialization causes both "" and null values to go to the frontend as null... // + // so we get 2 rows, but they look the same to the frontend. // + // turns out, users (probably?) don't care about the difference, so let's merge "" and null! // + ////////////////////////////////////////////////////////////////////////////////////////////////// + Integer rowsWithAValueToDecrease = mergeEmptyStringAndNull(field, fieldName, valueCounts, orderBy); + QFieldMetaData countField = new QFieldMetaData("count", QFieldType.INTEGER).withDisplayFormat(DisplayFormat.COMMAS).withLabel("Count"); QPossibleValueTranslator qPossibleValueTranslator = new QPossibleValueTranslator(); @@ -315,28 +337,49 @@ public class ColumnStatsStep implements BackendStep statsAggregateInput.withQueryJoin(queryJoin); } AggregateOutput statsAggregateOutput = new AggregateAction().execute(statsAggregateInput); - AggregateResult statsAggregateResult = statsAggregateOutput.getResults().get(0); + if(CollectionUtils.nullSafeHasContents(statsAggregateOutput.getResults())) + { + AggregateResult statsAggregateResult = statsAggregateOutput.getResults().get(0); - statsRecord.setValue(countNonNullField.getName(), statsAggregateResult.getAggregateValue(countNonNullAggregate)); - if(doCountDistinct) - { - statsRecord.setValue(countDistinctField.getName(), statsAggregateResult.getAggregateValue(countDistinctAggregate)); - } - if(doSum) - { - statsRecord.setValue(sumField.getName(), statsAggregateResult.getAggregateValue(sumAggregate)); - } - if(doAvg) - { - statsRecord.setValue(avgField.getName(), statsAggregateResult.getAggregateValue(avgAggregate)); - } - if(doMin) - { - statsRecord.setValue(minField.getName(), statsAggregateResult.getAggregateValue(minAggregate)); - } - if(doMax) - { - statsRecord.setValue(maxField.getName(), statsAggregateResult.getAggregateValue(maxAggregate)); + statsRecord.setValue(countNonNullField.getName(), statsAggregateResult.getAggregateValue(countNonNullAggregate)); + if(doCountDistinct) + { + statsRecord.setValue(countDistinctField.getName(), statsAggregateResult.getAggregateValue(countDistinctAggregate)); + } + if(doSum) + { + statsRecord.setValue(sumField.getName(), statsAggregateResult.getAggregateValue(sumAggregate)); + } + if(doAvg) + { + statsRecord.setValue(avgField.getName(), statsAggregateResult.getAggregateValue(avgAggregate)); + } + if(doMin) + { + statsRecord.setValue(minField.getName(), statsAggregateResult.getAggregateValue(minAggregate)); + } + if(doMax) + { + statsRecord.setValue(maxField.getName(), statsAggregateResult.getAggregateValue(maxAggregate)); + } + + if(rowsWithAValueToDecrease != null) + { + /////////////////////////////////////////////////////////////////////////////////////////////// + // this is in case we merged any "" and null values - // + // we need to take away however many ""'s there were from countNonNull (treat those as null) // + // and decrease unique values by 1 // + /////////////////////////////////////////////////////////////////////////////////////////////// + try + { + statsRecord.setValue(countNonNullField.getName(), statsRecord.getValueInteger(countNonNullField.getName()) - rowsWithAValueToDecrease); + statsRecord.setValue(countDistinctField.getName(), statsRecord.getValueInteger(countDistinctField.getName()) - 1); + } + catch(Exception e) + { + LOG.warn("Error decreasing by non-null empty string count", e, logPair("fieldName", fieldName), logPair("tableName", tableName)); + } + } } } @@ -354,4 +397,70 @@ public class ColumnStatsStep implements BackendStep } } + + + /******************************************************************************* + ** + *******************************************************************************/ + private Integer mergeEmptyStringAndNull(QFieldMetaData field, String fieldName, ArrayList valueCounts, String orderBy) + { + if(field.getType().isStringLike()) + { + Integer nullCount = null; + Integer emptyStringCount = null; + for(QRecord record : valueCounts) + { + if("".equals(record.getValue(fieldName))) + { + emptyStringCount = record.getValueInteger("count"); + } + else if(record.getValue(fieldName) == null) + { + nullCount = record.getValueInteger("count"); + } + } + + if(nullCount != null && emptyStringCount != null) + { + Iterator iterator = valueCounts.iterator(); + while(iterator.hasNext()) + { + QRecord record = iterator.next(); + if("".equals(record.getValue(fieldName))) + { + iterator.remove(); + } + else if(record.getValue(fieldName) == null) + { + record.setValue("count", nullCount + emptyStringCount); + } + } + + /////////////////////////////////////////////////// + // re-sort the records, as the counts may change // + /////////////////////////////////////////////////// + if(StringUtils.hasContent(orderBy)) + { + if(orderBy.toLowerCase().startsWith("count.")) + { + valueCounts.sort(Comparator.comparing(r -> r.getValueInteger("count"))); + } + else + { + valueCounts.sort(Comparator.comparing(r -> Objects.requireNonNullElse(r.getValueString(fieldName), ""))); + } + + if(orderBy.toLowerCase().endsWith(".desc")) + { + Collections.reverse(valueCounts); + } + } + + return (emptyStringCount); + } + } + + return (null); + } + } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModuleTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModuleTest.java index 23b8ab93..e533691e 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModuleTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/modules/backend/implementations/memory/MemoryBackendModuleTest.java @@ -22,12 +22,14 @@ package com.kingsrook.qqq.backend.core.modules.backend.implementations.memory; +import java.math.BigDecimal; import java.time.LocalDate; import java.time.Month; import java.util.List; import com.kingsrook.qqq.backend.core.BaseTest; import com.kingsrook.qqq.backend.core.actions.customizers.AbstractPostQueryCustomizer; import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizers; +import com.kingsrook.qqq.backend.core.actions.tables.AggregateAction; import com.kingsrook.qqq.backend.core.actions.tables.CountAction; import com.kingsrook.qqq.backend.core.actions.tables.DeleteAction; import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; @@ -35,6 +37,14 @@ import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.actions.tables.UpdateAction; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.Aggregate; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOperator; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateOutput; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.AggregateResult; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.GroupBy; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByAggregate; +import com.kingsrook.qqq.backend.core.model.actions.tables.aggregate.QFilterOrderByGroupBy; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteInput; import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteOutput; @@ -52,6 +62,7 @@ import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference; import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeUsage; +import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.utils.TestUtils; @@ -59,6 +70,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -514,6 +526,139 @@ class MemoryBackendModuleTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testAggregate() throws QException + { + InsertInput insertInput = new InsertInput(); + insertInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + insertInput.setRecords(List.of( + new QRecord().withValue("noOfShoes", 1).withValue("lastName", "Simpson"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", "Simpson"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", "Flanders"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", "Flanders"), + new QRecord().withValue("noOfShoes", 3).withValue("lastName", "Flanders"), + new QRecord().withValue("noOfShoes", null).withValue("lastName", "Flanders") + )); + new InsertAction().execute(insertInput); + + { + //////////////////////////////// + // do some integer aggregates // + //////////////////////////////// + AggregateInput aggregateInput = new AggregateInput(); + aggregateInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.SUM)); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.COUNT)); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.COUNT_DISTINCT)); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.MIN)); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.MAX)); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.AVG)); + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(1, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(10, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals(5, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.COUNT))); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.COUNT_DISTINCT))); + assertEquals(1, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.MIN))); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.MAX))); + assertEquals(new BigDecimal(2), aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.AVG))); + } + + { + /////////////////////////////// + // do some string aggregates // + /////////////////////////////// + AggregateInput aggregateInput = new AggregateInput(); + aggregateInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + aggregateInput.withAggregate(new Aggregate("lastName", AggregateOperator.COUNT)); + aggregateInput.withAggregate(new Aggregate("lastName", AggregateOperator.COUNT_DISTINCT)); + aggregateInput.withAggregate(new Aggregate("lastName", AggregateOperator.MIN)); + aggregateInput.withAggregate(new Aggregate("lastName", AggregateOperator.MAX)); + + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(1, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(6, aggregateResult.getAggregateValue(new Aggregate("lastName", AggregateOperator.COUNT))); + assertEquals(2, aggregateResult.getAggregateValue(new Aggregate("lastName", AggregateOperator.COUNT_DISTINCT))); + assertEquals("Flanders", aggregateResult.getAggregateValue(new Aggregate("lastName", AggregateOperator.MIN))); + assertEquals("Simpson", aggregateResult.getAggregateValue(new Aggregate("lastName", AggregateOperator.MAX))); + + assertThatThrownBy(() -> new AggregateAction().execute(aggregateInput.withAggregates(List.of(new Aggregate("lastName", AggregateOperator.SUM))))).hasStackTraceContaining("Cannot perform SUM"); + assertThatThrownBy(() -> new AggregateAction().execute(aggregateInput.withAggregates(List.of(new Aggregate("lastName", AggregateOperator.AVG))))).hasStackTraceContaining("Cannot perform AVG"); + } + + { + //////////////////// + // do a group-bys // + //////////////////// + AggregateInput aggregateInput = new AggregateInput(); + aggregateInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + aggregateInput.withAggregate(new Aggregate("noOfShoes", AggregateOperator.SUM)); + aggregateInput.withGroupBy(new GroupBy(QFieldType.STRING, "lastName")); + + { + aggregateInput.setFilter(new QQueryFilter().withOrderBy(new QFilterOrderByAggregate(new Aggregate("noOfShoes", AggregateOperator.SUM)))); + + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(2, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Simpson", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + + aggregateResult = aggregateOutput.getResults().get(1); + assertEquals(7, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Flanders", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + } + { + /////////////////////////////////////////////////////////////////////////// + // with all different versions of order-by (agg or groupBy, asc or desc) // + /////////////////////////////////////////////////////////////////////////// + aggregateInput.setFilter(new QQueryFilter().withOrderBy(new QFilterOrderByAggregate(new Aggregate("noOfShoes", AggregateOperator.SUM)).withIsAscending(false))); + + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(2, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(7, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Flanders", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + + aggregateResult = aggregateOutput.getResults().get(1); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Simpson", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + } + { + aggregateInput.setFilter(new QQueryFilter().withOrderBy(new QFilterOrderByGroupBy(new GroupBy(QFieldType.STRING, "lastName")))); + + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(2, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(7, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Flanders", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + + aggregateResult = aggregateOutput.getResults().get(1); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Simpson", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + } + { + aggregateInput.setFilter(new QQueryFilter().withOrderBy(new QFilterOrderByGroupBy(new GroupBy(QFieldType.STRING, "lastName")).withIsAscending(false))); + + AggregateOutput aggregateOutput = new AggregateAction().execute(aggregateInput); + assertEquals(2, aggregateOutput.getResults().size()); + AggregateResult aggregateResult = aggregateOutput.getResults().get(0); + assertEquals(3, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Simpson", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + + aggregateResult = aggregateOutput.getResults().get(1); + assertEquals(7, aggregateResult.getAggregateValue(new Aggregate("noOfShoes", AggregateOperator.SUM))); + assertEquals("Flanders", aggregateResult.getGroupByValue(new GroupBy(QFieldType.STRING, "lastName"))); + } + } + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStepTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStepTest.java new file mode 100644 index 00000000..0e7f9ed9 --- /dev/null +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/columnstats/ColumnStatsStepTest.java @@ -0,0 +1,61 @@ +package com.kingsrook.qqq.backend.core.processes.implementations.columnstats; + + +import java.io.Serializable; +import java.util.List; +import java.util.Map; +import com.kingsrook.qqq.backend.core.BaseTest; +import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; +import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepInput; +import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepOutput; +import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; +import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.utils.TestUtils; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; + + +/******************************************************************************* + ** Unit test for ColumnStatsStep + *******************************************************************************/ +class ColumnStatsStepTest extends BaseTest +{ + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testEmptyStringAndNullRollUpTogether() throws QException + { + InsertInput insertInput = new InsertInput(); + insertInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + insertInput.setRecords(List.of( + new QRecord().withValue("noOfShoes", 1).withValue("lastName", "Simpson"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", "Simpson"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", "Simpson"), + new QRecord().withValue("noOfShoes", 2).withValue("lastName", ""), // this record and the next one - + new QRecord().withValue("noOfShoes", 3).withValue("lastName", null), // this record and the previous - should both come out as null below + new QRecord().withValue("noOfShoes", null).withValue("lastName", "Flanders") + )); + new InsertAction().execute(insertInput); + + RunBackendStepInput input = new RunBackendStepInput(); + input.addValue("tableName", TestUtils.TABLE_NAME_PERSON_MEMORY); + input.addValue("fieldName", "lastName"); + input.addValue("orderBy", "count.desc"); + + RunBackendStepOutput output = new RunBackendStepOutput(); + new ColumnStatsStep().run(input, output); + + Map values = output.getValues(); + + @SuppressWarnings("unchecked") + List valueCounts = (List) values.get("valueCounts"); + + assertThat(valueCounts.get(0).getValues()).hasFieldOrPropertyWithValue("lastName", "Simpson").hasFieldOrPropertyWithValue("count", 3); + assertThat(valueCounts.get(1).getValues()).hasFieldOrPropertyWithValue("lastName", null).hasFieldOrPropertyWithValue("count", 2); // here's the assert for the "" and null record above. + assertThat(valueCounts.get(2).getValues()).hasFieldOrPropertyWithValue("lastName", "Flanders").hasFieldOrPropertyWithValue("count", 1); + } + +} \ No newline at end of file From 1db6dfb2adc30fbe0efb953a721926c6b85baae7 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Jun 2023 15:02:47 -0500 Subject: [PATCH 7/7] avoid calling auditDetail insert if no records --- .../qqq/backend/core/actions/audits/AuditAction.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/audits/AuditAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/audits/AuditAction.java index 7d6a77c0..4bbd6881 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/audits/AuditAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/audits/AuditAction.java @@ -215,10 +215,13 @@ public class AuditAction extends AbstractQActionFunction