From 58b0936c50ac10b6b06690b2335a83e4e3147475 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 14:41:40 -0500 Subject: [PATCH 01/11] Add details to Incorrect number of values given exception --- .../qqq/backend/module/rdbms/actions/AbstractRDBMSAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/AbstractRDBMSAction.java b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/AbstractRDBMSAction.java index f9e12361..501a45ba 100644 --- a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/AbstractRDBMSAction.java +++ b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/AbstractRDBMSAction.java @@ -661,7 +661,7 @@ public abstract class AbstractRDBMSAction } else if(!expectedNoOfParams.equals(values.size())) { - throw new IllegalArgumentException("Incorrect number of values given for criteria [" + field.getName() + "]"); + throw new IllegalArgumentException("Incorrect number of values given for criteria [" + field.getName() + "] (expected " + expectedNoOfParams + ", received " + values.size() + ")"); } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// From 2de3306f9522f72c160b92735638abec028d9eb6 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 14:41:55 -0500 Subject: [PATCH 02/11] Add c'tor that takes table name, and override withTableName --- .../tables/aggregate/AggregateInput.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/aggregate/AggregateInput.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/aggregate/AggregateInput.java index 9d69e5ac..e0fe22fc 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/aggregate/AggregateInput.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/aggregate/AggregateInput.java @@ -59,6 +59,30 @@ public class AggregateInput extends AbstractTableActionInput + /******************************************************************************* + ** Constructor + ** + *******************************************************************************/ + public AggregateInput(String tableName) + { + this(); + setTableName(tableName); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + public AggregateInput withTableName(String tableName) + { + super.withTableName(tableName); + return (this); + } + + + /******************************************************************************* ** Getter for filter ** From d92be4e69b4fddcff487c1c5a7a289e8bc75a366 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:00:36 -0500 Subject: [PATCH 03/11] don't duplicate apikey=value in re-tries; mask api key in outboundApiLog urls --- .../module/api/actions/BaseAPIActionUtil.java | 114 +++++++++++------- .../api/actions/BaseAPIActionUtilTest.java | 68 +++++++++++ 2 files changed, 138 insertions(+), 44 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 57cef42d..ae9fe8d0 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 @@ -734,20 +734,7 @@ public class BaseAPIActionUtil case API_KEY_HEADER -> request.setHeader("API-Key", backendMetaData.getApiKey()); case API_TOKEN -> request.setHeader("Authorization", "Token " + backendMetaData.getApiKey()); case OAUTH2 -> request.setHeader("Authorization", "Bearer " + getOAuth2Token()); - case API_KEY_QUERY_PARAM -> - { - try - { - String uri = request.getURI().toString(); - uri += (uri.contains("?") ? "&" : "?"); - uri += backendMetaData.getApiKeyQueryParamName() + "=" + backendMetaData.getApiKey(); - request.setURI(new URI(uri)); - } - catch(URISyntaxException e) - { - throw (new QException("Error setting authorization query parameter", e)); - } - } + case API_KEY_QUERY_PARAM -> addApiKeyQueryParamToRequest(request); case CUSTOM -> { handleCustomAuthorization(request); @@ -758,6 +745,35 @@ public class BaseAPIActionUtil + /*************************************************************************** + ** + ***************************************************************************/ + protected void addApiKeyQueryParamToRequest(HttpRequestBase request) throws QException + { + try + { + String uri = request.getURI().toString(); + String pair = backendMetaData.getApiKeyQueryParamName() + "=" + backendMetaData.getApiKey(); + + /////////////////////////////////////////////////////////////////////////////////// + // avoid re-adding the name=value pair if it's already there (e.g., for a retry) // + /////////////////////////////////////////////////////////////////////////////////// + if(!uri.contains(pair)) + { + uri += (uri.contains("?") ? "&" : "?"); + uri += pair; + } + + request.setURI(new URI(uri)); + } + catch(URISyntaxException e) + { + throw (new QException("Error setting authorization query parameter", e)); + } + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -1207,39 +1223,11 @@ public class BaseAPIActionUtil return; } - String requestBody = null; - if(request instanceof HttpEntityEnclosingRequest entityRequest) - { - try - { - requestBody = StringUtils.join("\n", IOUtils.readLines(entityRequest.getEntity().getContent())); - } - catch(Exception e) - { - // leave it null... - } - } - - //////////////////////////////////// - // mask api keys in query strings // - //////////////////////////////////// - String url = request.getURI().toString(); - if(backendMetaData.getAuthorizationType().equals(AuthorizationType.API_KEY_QUERY_PARAM)) - { - url = url.replaceFirst(backendMetaData.getApiKey(), "******"); - } + OutboundAPILog outboundAPILog = generateOutboundApiLogRecord(request, response); InsertInput insertInput = new InsertInput(); insertInput.setTableName(table.getName()); - insertInput.setRecords(List.of(new OutboundAPILog() - .withMethod(request.getMethod()) - .withUrl(url) - .withTimestamp(Instant.now()) - .withRequestBody(requestBody) - .withStatusCode(response.getStatusCode()) - .withResponseBody(response.getContent()) - .toQRecord() - )); + insertInput.setRecords(List.of(outboundAPILog.toQRecord())); new InsertAction().executeAsync(insertInput); } catch(Exception e) @@ -1250,6 +1238,44 @@ public class BaseAPIActionUtil + /*************************************************************************** + ** + ***************************************************************************/ + public OutboundAPILog generateOutboundApiLogRecord(HttpRequestBase request, QHttpResponse response) + { + String requestBody = null; + if(request instanceof HttpEntityEnclosingRequest entityRequest) + { + try + { + requestBody = StringUtils.join("\n", IOUtils.readLines(entityRequest.getEntity().getContent(), StandardCharsets.UTF_8)); + } + catch(Exception e) + { + // leave it null... + } + } + + //////////////////////////////////// + // mask api keys in query strings // + //////////////////////////////////// + String url = request.getURI().toString(); + if(backendMetaData.getAuthorizationType().equals(AuthorizationType.API_KEY_QUERY_PARAM)) + { + url = url.replaceAll(backendMetaData.getApiKey(), "******"); + } + + return new OutboundAPILog() + .withMethod(request.getMethod()) + .withUrl(url) + .withTimestamp(Instant.now()) + .withRequestBody(requestBody) + .withStatusCode(response.getStatusCode()) + .withResponseBody(response.getContent()); + } + + + /******************************************************************************* ** *******************************************************************************/ 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 60923ea5..ad985295 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 @@ -61,6 +61,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.tables.UniqueKey; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.SleepUtils; +import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeConsumer; import com.kingsrook.qqq.backend.module.api.BaseTest; import com.kingsrook.qqq.backend.module.api.TestUtils; import com.kingsrook.qqq.backend.module.api.exceptions.RateLimitException; @@ -74,6 +75,7 @@ import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpRequestBase; import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; @@ -868,6 +870,72 @@ class BaseAPIActionUtilTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testAuthorizationApiKeyQueryParam() throws QException + { + APIBackendMetaData backend = (APIBackendMetaData) QContext.getQInstance().getBackend(TestUtils.MOCK_BACKEND_NAME); + backend.setAuthorizationType(AuthorizationType.API_KEY_QUERY_PARAM); + backend.setApiKeyQueryParamName("apikey"); + backend.setApiKey("9876-WXYZ"); + + //////////////////////////////////////////////////////////////////////////////////////////// + // this will make it not use the mock makeRequest method, // + // but instead the mock executeHttpRequest, so we can test code from the base makeRequest // + //////////////////////////////////////////////////////////////////////////////////////////// + mockApiUtilsHelper.setUseMock(false); + + ////////////////////////////////////////////////////////////////////////////// + // we'll want to assert that the URL has the api query string - and just // + // one copy of it (as we once had a bug where it got duplicated upon retry) // + ////////////////////////////////////////////////////////////////////////////// + UnsafeConsumer asserter = request -> assertThat(request.getURI().toString()) + .contains("?apikey=9876-WXYZ") + .doesNotContain("?apikey=9876-WXYZ&apikey=9876-WXYZ"); + + //////////////////////////////////////// + // queue up a 429, so we'll try-again // + //////////////////////////////////////// + mockApiUtilsHelper.setMockRequestAsserter(asserter); + mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(429).withContent("")); + + ////////////////////// + // queue a response // + ////////////////////// + mockApiUtilsHelper.setMockRequestAsserter(asserter); + mockApiUtilsHelper.enqueueMockResponse(""" + {"id": 3, "name": "Bart"}, + """); + + GetOutput getOutput = runSimpleGetAction(); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testGenerateOutboundApiLogRecord() throws QException + { + APIBackendMetaData backend = (APIBackendMetaData) QContext.getQInstance().getBackend(TestUtils.MOCK_BACKEND_NAME); + backend.setAuthorizationType(AuthorizationType.API_KEY_QUERY_PARAM); + backend.setApiKeyQueryParamName("apikey"); + backend.setApiKey("9876-WXYZ"); + + MockApiActionUtils mockApiActionUtils = new MockApiActionUtils(); + mockApiActionUtils.setBackendMetaData(backend); + OutboundAPILog outboundAPILog = mockApiActionUtils.generateOutboundApiLogRecord(new HttpGet("...?apikey=9876-WXYZ"), new QHttpResponse()); + + assertThat(outboundAPILog.getUrl()) + .doesNotContain("9876-WXYZ") + .contains("?apikey=*****"); + } + + + /******************************************************************************* ** *******************************************************************************/ From 26fc4fb4e043045223638cabef65561360a27d3c Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:01:22 -0500 Subject: [PATCH 04/11] Initial checkin --- .../qqq/backend/core/utils/CountingHash.java | 126 ++++++++++++++++++ .../backend/core/utils/CountingHashTest.java | 76 +++++++++++ 2 files changed, 202 insertions(+) create mode 100755 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/utils/CountingHash.java create mode 100644 qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/CountingHashTest.java diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/utils/CountingHash.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/utils/CountingHash.java new file mode 100755 index 00000000..8db0e364 --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/utils/CountingHash.java @@ -0,0 +1,126 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. 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.utils; + + +import java.io.Serializable; +import java.util.AbstractMap; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import com.kingsrook.qqq.backend.core.utils.collections.MutableMap; + + +/******************************************************************************* + ** Hash that provides "counting" capability -- keys map to Integers that + ** are automatically/easily summed to + ** + *******************************************************************************/ +public class CountingHash extends AbstractMap implements Serializable +{ + private Map map = null; + + + + /******************************************************************************* + ** Default constructor + ** + *******************************************************************************/ + public CountingHash() + { + this.map = new HashMap<>(); + } + + + + /******************************************************************************* + ** Constructor where you can supply a source map (e.g., if you want a specific + ** Map type (like LinkedHashMap), or with pre-values. + ** + ** Note - the input map will be wrapped in a MutableMap - so - it'll be mutable. + ** + *******************************************************************************/ + public CountingHash(Map sourceMap) + { + this.map = new MutableMap<>(sourceMap); + } + + + + /******************************************************************************* + ** Increment the value for the specified key by 1. + ** + *******************************************************************************/ + public Integer add(K key) + { + Integer value = getOrCreateListForKey(key); + Integer sum = value + 1; + map.put(key, sum); + return (sum); + } + + + + /******************************************************************************* + ** Increment the value for the specified key by the supplied addend + ** + *******************************************************************************/ + public Integer add(K key, Integer addend) + { + Integer value = getOrCreateListForKey(key); + Integer sum = value + addend; + map.put(key, sum); + return (sum); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private Integer getOrCreateListForKey(K key) + { + Integer value; + + if(!this.map.containsKey(key)) + { + this.map.put(key, 0); + value = 0; + } + else + { + value = this.map.get(key); + } + return value; + } + + + + /*************************************************************************** + * + ***************************************************************************/ + public Set> entrySet() + { + return this.map.entrySet(); + } + +} diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/CountingHashTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/CountingHashTest.java new file mode 100644 index 00000000..7033b170 --- /dev/null +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/CountingHashTest.java @@ -0,0 +1,76 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. 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.utils; + + +import java.util.Map; +import com.kingsrook.qqq.backend.core.BaseTest; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + + +/******************************************************************************* + ** Unit test for CountingHash + *******************************************************************************/ +class CountingHashTest extends BaseTest +{ + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void test() + { + CountingHash countingHash = new CountingHash<>(); + + assertNull(countingHash.get("a")); + + countingHash.add("a"); + assertEquals(1, countingHash.get("a")); + + countingHash.add("a"); + assertEquals(2, countingHash.get("a")); + + countingHash.add("a", 2); + assertEquals(4, countingHash.get("a")); + + countingHash.add("b", 5); + assertEquals(5, countingHash.get("b")); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testAlwaysMutable() + { + CountingHash alwaysMutable = new CountingHash<>(Map.of("A", 5)); + alwaysMutable.add("A"); + alwaysMutable.add("B"); + assertEquals(6, alwaysMutable.get("A")); + assertEquals(1, alwaysMutable.get("B")); + } + +} \ No newline at end of file From 09c4d9961244a21fee06ad511212f42113437ad9 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:02:06 -0500 Subject: [PATCH 05/11] Avoid NPE and return w/ noop in performValidations if null (or empty) input records --- .../qqq/backend/core/actions/tables/InsertAction.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/InsertAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/InsertAction.java index 9773d3f0..9336f9db 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/InsertAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/InsertAction.java @@ -227,6 +227,11 @@ public class InsertAction extends AbstractQActionFunction Date: Thu, 1 Aug 2024 15:02:20 -0500 Subject: [PATCH 06/11] Add a log info re: releasing lock --- .../qqq/backend/core/processes/locks/ProcessLockUtils.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/locks/ProcessLockUtils.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/locks/ProcessLockUtils.java index fdc5b20e..ac3a05e8 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/locks/ProcessLockUtils.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/locks/ProcessLockUtils.java @@ -433,6 +433,8 @@ public class ProcessLockUtils { throw (new QException("Error deleting processLock record: " + deleteOutput.getRecordsWithErrors().get(0).getErrorsAsString())); } + + LOG.info("Released process lock", logPair("id", processLock.getId()), logPair("key", processLock.getKey()), logPair("typeId", processLock.getProcessLockTypeId()), logPair("details", processLock.getDetails())); } catch(QException e) { From 5aed59b9b1fc5c65f2f1f9ca82827308a98b1abd Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:02:38 -0500 Subject: [PATCH 07/11] Add implements AutoCloseable, so we could use in a try-with-resources --- .../kingsrook/qqq/backend/core/actions/QBackendTransaction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/QBackendTransaction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/QBackendTransaction.java index 465dae45..feab34a6 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/QBackendTransaction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/QBackendTransaction.java @@ -37,7 +37,7 @@ import com.kingsrook.qqq.backend.core.modules.backend.QBackendModuleInterface; ** ** Note: One would imagine that this class shouldn't ever implement Serializable... *******************************************************************************/ -public class QBackendTransaction +public class QBackendTransaction implements AutoCloseable { /******************************************************************************* From d44790545d7d11db2e41fd986b5f0b3537c91ad2 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:04:03 -0500 Subject: [PATCH 08/11] Add total # failures to message; remove unused c'tor --- .../QInstanceValidationException.java | 30 ++++--------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/exceptions/QInstanceValidationException.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/exceptions/QInstanceValidationException.java index fa090ca1..8b44cf4c 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/exceptions/QInstanceValidationException.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/exceptions/QInstanceValidationException.java @@ -22,8 +22,8 @@ package com.kingsrook.qqq.backend.core.exceptions; -import java.util.Arrays; import java.util.List; +import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; @@ -55,12 +55,11 @@ public class QInstanceValidationException extends QException *******************************************************************************/ public QInstanceValidationException(List reasons) { - super( - (reasons != null && reasons.size() > 0) - ? "Instance validation failed for the following reasons:\n - " + StringUtils.join("\n - ", reasons) - : "Validation failed, but no reasons were provided"); + super((CollectionUtils.nullSafeHasContents(reasons)) + ? "Instance validation failed for the following reasons:\n - " + StringUtils.join("\n - ", reasons) + "\n(" + reasons.size() + " Total reason" + StringUtils.plural(reasons) + ")" + : "Validation failed, but no reasons were provided"); - if(reasons != null && reasons.size() > 0) + if(CollectionUtils.nullSafeHasContents(reasons)) { this.reasons = reasons; } @@ -68,25 +67,6 @@ public class QInstanceValidationException extends QException - /******************************************************************************* - ** Constructor of an array/varargs of reasons. They feed into the core exception message. - ** - *******************************************************************************/ - public QInstanceValidationException(String... reasons) - { - super( - (reasons != null && reasons.length > 0) - ? "Instance validation failed for the following reasons: " + StringUtils.joinWithCommasAndAnd(Arrays.stream(reasons).toList()) - : "Validation failed, but no reasons were provided"); - - if(reasons != null && reasons.length > 0) - { - this.reasons = Arrays.stream(reasons).toList(); - } - } - - - /******************************************************************************* ** Constructor of message & cause - does not populate reasons! ** From ba3cf53c3094e85beb2b4a5b128cfb0056765792 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:08:53 -0500 Subject: [PATCH 09/11] Update to throw QNotFoundException if view isn't found by id (rather than NPE) --- .../savedviews/QuerySavedViewProcess.java | 14 +++- .../savedviews/SavedViewProcessTests.java | 65 +++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/QuerySavedViewProcess.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/QuerySavedViewProcess.java index f4f57516..4ac4038a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/QuerySavedViewProcess.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/QuerySavedViewProcess.java @@ -29,6 +29,7 @@ import com.kingsrook.qqq.backend.core.actions.processes.BackendStep; import com.kingsrook.qqq.backend.core.actions.tables.GetAction; import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.exceptions.QNotFoundException; import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepInput; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepOutput; @@ -44,6 +45,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference; import com.kingsrook.qqq.backend.core.model.metadata.processes.QBackendStepMetaData; import com.kingsrook.qqq.backend.core.model.metadata.processes.QProcessMetaData; import com.kingsrook.qqq.backend.core.model.savedviews.SavedView; +import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; /******************************************************************************* @@ -78,10 +80,10 @@ public class QuerySavedViewProcess implements BackendStep public void run(RunBackendStepInput runBackendStepInput, RunBackendStepOutput runBackendStepOutput) throws QException { ActionHelper.validateSession(runBackendStepInput); + Integer savedViewId = runBackendStepInput.getValueInteger("id"); try { - Integer savedViewId = runBackendStepInput.getValueInteger("id"); if(savedViewId != null) { GetInput input = new GetInput(); @@ -89,6 +91,11 @@ public class QuerySavedViewProcess implements BackendStep input.setPrimaryKey(savedViewId); GetOutput output = new GetAction().execute(input); + if(output.getRecord() == null) + { + throw (new QNotFoundException("The requested view was not found.")); + } + runBackendStepOutput.addRecord(output.getRecord()); runBackendStepOutput.addValue("savedView", output.getRecord()); runBackendStepOutput.addValue("savedViewList", (Serializable) List.of(output.getRecord())); @@ -108,6 +115,11 @@ public class QuerySavedViewProcess implements BackendStep runBackendStepOutput.addValue("savedViewList", (Serializable) output.getRecords()); } } + catch(QNotFoundException qnfe) + { + LOG.info("View not found", logPair("savedViewId", savedViewId)); + throw (qnfe); + } catch(Exception e) { LOG.warn("Error querying for saved views", e); diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/SavedViewProcessTests.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/SavedViewProcessTests.java index f4474bef..304dc425 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/SavedViewProcessTests.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/processes/implementations/savedviews/SavedViewProcessTests.java @@ -82,7 +82,7 @@ class SavedViewProcessTests extends BaseTest runProcessInput.addValue("tableName", tableName); runProcessInput.addValue("viewJson", JsonUtils.toJson(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 47)))); RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); - List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); + List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); assertEquals(1, savedViewList.size()); savedViewId = savedViewList.get(0).getValueInteger("id"); assertNotNull(savedViewId); @@ -103,7 +103,7 @@ class SavedViewProcessTests extends BaseTest runProcessInput.setProcessName(QuerySavedViewProcess.getProcessMetaData().getName()); runProcessInput.addValue("tableName", tableName); RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); - List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); + List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); assertEquals(1, savedViewList.size()); assertEquals(1, savedViewList.get(0).getValueInteger("id")); assertEquals("My View", savedViewList.get(0).getValueString("label")); @@ -120,7 +120,7 @@ class SavedViewProcessTests extends BaseTest runProcessInput.addValue("tableName", tableName); runProcessInput.addValue("viewJson", JsonUtils.toJson(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 47)))); RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); - List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); + List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); assertEquals(1, savedViewList.size()); assertEquals(1, savedViewList.get(0).getValueInteger("id")); assertEquals("My Updated View", savedViewList.get(0).getValueString("label")); @@ -151,7 +151,7 @@ class SavedViewProcessTests extends BaseTest runProcessInput.addValue("label", "My Updated View"); runProcessInput.addValue("tableName", tableName); runProcessInput.addValue("viewJson", JsonUtils.toJson(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 47)))); - + ////////////////////////////////////////// // should throw a "duplicate" exception // ////////////////////////////////////////// @@ -183,7 +183,64 @@ class SavedViewProcessTests extends BaseTest RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); assertEquals(0, ((List) runProcessOutput.getValues().get("savedViewList")).size()); } + } + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testNotFoundThrowsProperly() throws QException + { + QInstance qInstance = QContext.getQInstance(); + new SavedViewsMetaDataProvider().defineAll(qInstance, TestUtils.MEMORY_BACKEND_NAME, null); + String tableName = TestUtils.TABLE_NAME_PERSON_MEMORY; + + { + //////////////////////////////////////////////////////// + // get one by id when it doesn't exist - should throw // + //////////////////////////////////////////////////////// + RunProcessInput runProcessInput = new RunProcessInput(); + runProcessInput.setProcessName(QuerySavedViewProcess.getProcessMetaData().getName()); + runProcessInput.addValue("tableName", tableName); + runProcessInput.addValue("id", -1); + assertThatThrownBy(() -> new RunProcessAction().execute(runProcessInput)) + .hasMessageContaining("view was not found") + .isInstanceOf(QUserFacingException.class); + } + + Integer savedViewId; + { + ////////////////////// + // store a new view // + ////////////////////// + RunProcessInput runProcessInput = new RunProcessInput(); + runProcessInput.setProcessName(StoreSavedViewProcess.getProcessMetaData().getName()); + runProcessInput.addValue("label", "My View"); + runProcessInput.addValue("tableName", tableName); + runProcessInput.addValue("viewJson", JsonUtils.toJson(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 47)))); + RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); + List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); + assertEquals(1, savedViewList.size()); + savedViewId = savedViewList.get(0).getValueInteger("id"); + assertNotNull(savedViewId); + } + + { + //////////////////////////////////////// + // get now with valid id, should work // + //////////////////////////////////////// + RunProcessInput runProcessInput = new RunProcessInput(); + runProcessInput.setProcessName(QuerySavedViewProcess.getProcessMetaData().getName()); + runProcessInput.addValue("tableName", tableName); + runProcessInput.addValue("id", savedViewId); + RunProcessOutput runProcessOutput = new RunProcessAction().execute(runProcessInput); + List savedViewList = (List) runProcessOutput.getValues().get("savedViewList"); + assertEquals(1, savedViewList.size()); + assertEquals(1, savedViewList.get(0).getValueInteger("id")); + assertEquals("My View", savedViewList.get(0).getValueString("label")); + } } } \ No newline at end of file From a11d584c8a10ce9ea817eb5048688145bb730a8d Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:11:20 -0500 Subject: [PATCH 10/11] Fix formatting of booleans when value is string (e.g., format based on QFieldMetaData type, not value object class) --- .../backend/core/actions/values/QValueFormatter.java | 12 +++++++++--- .../core/actions/values/QValueFormatterTest.java | 6 +++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatter.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatter.java index fab61595..d5704252 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatter.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatter.java @@ -68,7 +68,7 @@ public class QValueFormatter *******************************************************************************/ public static String formatValue(QFieldMetaData field, Serializable value) { - return (formatValue(field.getDisplayFormat(), field.getName(), value)); + return (formatValue(field.getDisplayFormat(), field.getType(), field.getName(), value)); } @@ -78,7 +78,7 @@ public class QValueFormatter *******************************************************************************/ public static String formatValue(String displayFormat, Serializable value) { - return (formatValue(displayFormat, "", value)); + return (formatValue(displayFormat, null, "", value)); } @@ -87,7 +87,7 @@ public class QValueFormatter ** For a display format string, an optional fieldName (only used for logging), ** and a value, apply the format. *******************************************************************************/ - private static String formatValue(String displayFormat, String fieldName, Serializable value) + private static String formatValue(String displayFormat, QFieldType fieldType, String fieldName, Serializable value) { ////////////////////////////////// // null values get null results // @@ -107,6 +107,11 @@ public class QValueFormatter return formatBoolean(b); } + if(QFieldType.BOOLEAN.equals(fieldType)) + { + return formatBoolean(ValueUtils.getValueAsBoolean(value)); + } + if(value instanceof LocalTime lt) { return formatLocalTime(lt); @@ -404,6 +409,7 @@ public class QValueFormatter } + /******************************************************************************* ** For a single record, set its display values - where caller (meant to stay private) ** can specify if they've already done fieldBehaviors (to avoid re-doing). diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatterTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatterTest.java index 8760b0fb..cd678974 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatterTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/values/QValueFormatterTest.java @@ -36,10 +36,10 @@ import com.kingsrook.qqq.backend.core.BaseTest; import com.kingsrook.qqq.backend.core.context.QContext; 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.fields.DateTimeDisplayValueBehavior; import com.kingsrook.qqq.backend.core.model.metadata.fields.DisplayFormat; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType; -import com.kingsrook.qqq.backend.core.model.metadata.fields.DateTimeDisplayValueBehavior; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.TestUtils; import org.junit.jupiter.api.Test; @@ -89,6 +89,10 @@ class QValueFormatterTest extends BaseTest assertNull(QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.BOOLEAN), null)); assertEquals("Yes", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.BOOLEAN), true)); assertEquals("No", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.BOOLEAN), false)); + assertEquals("Yes", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.BOOLEAN), "true")); + assertEquals("No", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.BOOLEAN), "false")); + assertEquals("true", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.STRING), "true")); + assertEquals("false", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.STRING), "false")); assertNull(QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.TIME), null)); assertEquals("5:00:00 AM", QValueFormatter.formatValue(new QFieldMetaData().withType(QFieldType.TIME), LocalTime.of(5, 0))); From 3eae3a5758c2d2d0cb5bd1fd0ad96cb6aa2f9284 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 1 Aug 2024 15:12:59 -0500 Subject: [PATCH 11/11] re-set queryStat startTimestamp to just before executeQuery, to avoid including time spent aquiring db connection --- .../module/rdbms/actions/RDBMSAggregateAction.java | 9 +++++++++ .../backend/module/rdbms/actions/RDBMSCountAction.java | 9 +++++++++ .../backend/module/rdbms/actions/RDBMSQueryAction.java | 9 +++++++++ 3 files changed, 27 insertions(+) diff --git a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSAggregateAction.java b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSAggregateAction.java index b174373b..7e231765 100644 --- a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSAggregateAction.java +++ b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSAggregateAction.java @@ -25,6 +25,7 @@ package com.kingsrook.qqq.backend.module.rdbms.actions; import java.io.Serializable; import java.sql.Connection; import java.sql.ResultSet; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -116,6 +117,14 @@ public class RDBMSAggregateAction extends AbstractRDBMSAction implements Aggrega actionTimeoutHelper = new ActionTimeoutHelper(aggregateInput.getTimeoutSeconds(), TimeUnit.SECONDS, new StatementTimeoutCanceller(statement, sql)); actionTimeoutHelper.start(); + /////////////////////////////////////////////////////////////////////////////////////////////////// + // to avoid counting time spent acquiring a connection, re-set the queryStat startTimestamp here // + /////////////////////////////////////////////////////////////////////////////////////////////////// + if(queryStat != null) + { + queryStat.setStartTimestamp(Instant.now()); + } + QueryManager.executeStatement(statement, sql, ((ResultSet resultSet) -> { ///////////////////////////////////////////////////////////////////////// diff --git a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountAction.java b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountAction.java index a24890f1..c9f87e8a 100644 --- a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountAction.java +++ b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountAction.java @@ -25,6 +25,7 @@ package com.kingsrook.qqq.backend.module.rdbms.actions; import java.io.Serializable; import java.sql.Connection; import java.sql.ResultSet; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -96,6 +97,14 @@ public class RDBMSCountAction extends AbstractRDBMSAction implements CountInterf actionTimeoutHelper = new ActionTimeoutHelper(countInput.getTimeoutSeconds(), TimeUnit.SECONDS, new StatementTimeoutCanceller(statement, sql)); actionTimeoutHelper.start(); + /////////////////////////////////////////////////////////////////////////////////////////////////// + // to avoid counting time spent acquiring a connection, re-set the queryStat startTimestamp here // + /////////////////////////////////////////////////////////////////////////////////////////////////// + if(queryStat != null) + { + queryStat.setStartTimestamp(Instant.now()); + } + QueryManager.executeStatement(statement, sql, ((ResultSet resultSet) -> { ///////////////////////////////////////////////////////////////////////// diff --git a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java index 5b687cd7..47305b81 100644 --- a/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java +++ b/qqq-backend-module-rdbms/src/main/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryAction.java @@ -28,6 +28,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; +import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -165,6 +166,14 @@ public class RDBMSQueryAction extends AbstractRDBMSAction implements QueryInterf actionTimeoutHelper = new ActionTimeoutHelper(queryInput.getTimeoutSeconds(), TimeUnit.SECONDS, new StatementTimeoutCanceller(statement, sql)); actionTimeoutHelper.start(); + /////////////////////////////////////////////////////////////////////////////////////////////////// + // to avoid counting time spent acquiring a connection, re-set the queryStat startTimestamp here // + /////////////////////////////////////////////////////////////////////////////////////////////////// + if(queryStat != null) + { + queryStat.setStartTimestamp(Instant.now()); + } + ////////////////////////////////////////////// // execute the query - iterate over results // //////////////////////////////////////////////