From f457fd0860865f828f1478b8c79e7a694006501a Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 3 Sep 2024 22:01:07 -0500 Subject: [PATCH 01/17] CE-1654 activate chickenAndEggSession while calling customizer.finalCustomizeSession --- .../implementations/Auth0AuthenticationModule.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java index 362c9415..baa1be72 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java @@ -313,7 +313,11 @@ public class Auth0AuthenticationModule implements QAuthenticationModuleInterface ////////////////////////////////////////////////////////////// if(getCustomizer() != null) { - getCustomizer().finalCustomizeSession(qInstance, qSession); + QContext.withTemporaryContext(QContext.capture(), () -> + { + QContext.setQSession(getChickenAndEggSession()); + getCustomizer().finalCustomizeSession(qInstance, qSession); + }); } return (qSession); From 9bf9825132f2caf98b379ada2baa6f46d56eabe2 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 5 Sep 2024 18:33:37 -0500 Subject: [PATCH 02/17] Option (turned on by default, controlled via javalin metadata) to not allow query requests without a limit --- .../javalin/QJavalinImplementation.java | 27 +++++ .../qqq/backend/javalin/QJavalinMetaData.java | 98 +++++++++++++++++ .../javalin/QJavalinImplementationTest.java | 100 +++++++++++++++++- .../qqq/backend/javalin/QJavalinTestBase.java | 2 +- 4 files changed, 225 insertions(+), 2 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 33c2b5ad..cca918ab 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 @@ -1279,6 +1279,11 @@ public class QJavalinImplementation queryInput.getFilter().setLimit(limit); } + if(queryInput.getFilter() == null || queryInput.getFilter().getLimit() == null) + { + handleQueryNullLimit(context, queryInput); + } + List queryJoins = processQueryJoinsParam(context); queryInput.setQueryJoins(queryJoins); @@ -1299,6 +1304,28 @@ public class QJavalinImplementation + /*************************************************************************** + ** + ***************************************************************************/ + private static void handleQueryNullLimit(Context context, QueryInput queryInput) + { + boolean allowed = javalinMetaData.getQueryWithoutLimitAllowed(); + if(!allowed) + { + if(queryInput.getFilter() == null) + { + queryInput.setFilter(new QQueryFilter()); + } + + queryInput.getFilter().setLimit(javalinMetaData.getQueryWithoutLimitDefault()); + LOG.log(javalinMetaData.getQueryWithoutLimitLogLevel(), "Query request did not specify a limit, which is not allowed. Using default instead", null, + logPair("defaultLimit", javalinMetaData.getQueryWithoutLimitDefault()), + logPair("path", context.path())); + } + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinMetaData.java b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinMetaData.java index e55db75c..89f422b2 100644 --- a/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinMetaData.java +++ b/qqq-middleware-javalin/src/main/java/com/kingsrook/qqq/backend/javalin/QJavalinMetaData.java @@ -23,6 +23,7 @@ package com.kingsrook.qqq.backend.javalin; import java.util.function.Function; +import org.apache.logging.log4j.Level; /******************************************************************************* @@ -36,6 +37,10 @@ public class QJavalinMetaData private Function logFilter; + private boolean queryWithoutLimitAllowed = false; + private Integer queryWithoutLimitDefault = 1000; + private Level queryWithoutLimitLogLevel = Level.INFO; + /******************************************************************************* @@ -143,4 +148,97 @@ public class QJavalinMetaData return (this); } + + + /******************************************************************************* + ** Getter for queryWithoutLimitAllowed + *******************************************************************************/ + public boolean getQueryWithoutLimitAllowed() + { + return (this.queryWithoutLimitAllowed); + } + + + + /******************************************************************************* + ** Setter for queryWithoutLimitAllowed + *******************************************************************************/ + public void setQueryWithoutLimitAllowed(boolean queryWithoutLimitAllowed) + { + this.queryWithoutLimitAllowed = queryWithoutLimitAllowed; + } + + + + /******************************************************************************* + ** Fluent setter for queryWithoutLimitAllowed + *******************************************************************************/ + public QJavalinMetaData withQueryWithoutLimitAllowed(boolean queryWithoutLimitAllowed) + { + this.queryWithoutLimitAllowed = queryWithoutLimitAllowed; + return (this); + } + + + + /******************************************************************************* + ** Getter for queryWithoutLimitDefault + *******************************************************************************/ + public Integer getQueryWithoutLimitDefault() + { + return (this.queryWithoutLimitDefault); + } + + + + /******************************************************************************* + ** Setter for queryWithoutLimitDefault + *******************************************************************************/ + public void setQueryWithoutLimitDefault(Integer queryWithoutLimitDefault) + { + this.queryWithoutLimitDefault = queryWithoutLimitDefault; + } + + + + /******************************************************************************* + ** Fluent setter for queryWithoutLimitDefault + *******************************************************************************/ + public QJavalinMetaData withQueryWithoutLimitDefault(Integer queryWithoutLimitDefault) + { + this.queryWithoutLimitDefault = queryWithoutLimitDefault; + return (this); + } + + + + /******************************************************************************* + ** Getter for queryWithoutLimitLogLevel + *******************************************************************************/ + public Level getQueryWithoutLimitLogLevel() + { + return (this.queryWithoutLimitLogLevel); + } + + + + /******************************************************************************* + ** Setter for queryWithoutLimitLogLevel + *******************************************************************************/ + public void setQueryWithoutLimitLogLevel(Level queryWithoutLimitLogLevel) + { + this.queryWithoutLimitLogLevel = queryWithoutLimitLogLevel; + } + + + + /******************************************************************************* + ** Fluent setter for queryWithoutLimitLogLevel + *******************************************************************************/ + public QJavalinMetaData withQueryWithoutLimitLogLevel(Level queryWithoutLimitLogLevel) + { + this.queryWithoutLimitLogLevel = queryWithoutLimitLogLevel; + return (this); + } + } 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 9f59d373..df633b30 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 @@ -33,6 +33,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Function; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.exceptions.QInstanceValidationException; +import com.kingsrook.qqq.backend.core.logging.QCollectingLogger; +import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.actions.reporting.ReportFormat; import com.kingsrook.qqq.backend.core.model.dashboard.widgets.WidgetType; import com.kingsrook.qqq.backend.core.model.metadata.QBackendMetaData; @@ -45,6 +47,7 @@ import com.kingsrook.qqq.backend.core.utils.JsonUtils; import com.kingsrook.qqq.backend.core.utils.SleepUtils; import kong.unirest.HttpResponse; import kong.unirest.Unirest; +import org.apache.logging.log4j.Level; import org.eclipse.jetty.http.HttpStatus; import org.json.JSONArray; import org.json.JSONObject; @@ -469,6 +472,101 @@ class QJavalinImplementationTest extends QJavalinTestBase + /******************************************************************************* + ** test a table query using an actual filter via POST, with no limit specified, + ** and with that not being allowed. + ** + *******************************************************************************/ + @Test + public void test_dataQueryWithFilterPOSTWithoutLimitNotAllowed() throws QInstanceValidationException + { + try + { + qJavalinImplementation.getJavalinMetaData() + .withQueryWithoutLimitAllowed(false) + .withQueryWithoutLimitDefault(3) + .withQueryWithoutLimitLogLevel(Level.WARN); + + QCollectingLogger collectingLogger = QLogger.activateCollectingLoggerForClass(QJavalinImplementation.class); + + String filterJson = """ + {"criteria":[]}"""; + + HttpResponse response = Unirest.post(BASE_URL + "/data/person/query") + .field("filter", filterJson) + .asString(); + + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertTrue(jsonObject.has("records")); + JSONArray records = jsonObject.getJSONArray("records"); + assertEquals(3, records.length()); + + assertThat(collectingLogger.getCollectedMessages()) + .anyMatch(m -> m.getLevel().equals(Level.WARN) && m.getMessage().contains("Query request did not specify a limit")); + } + finally + { + QLogger.activateCollectingLoggerForClass(QJavalinImplementation.class); + resetMetaDataQueryWithoutLimitSettings(); + } + } + + + + /******************************************************************************* + ** test a table query using an actual filter via POST, with no limit specified, + ** but with that being allowed. + ** + *******************************************************************************/ + @Test + public void test_dataQueryWithFilterPOSTWithoutLimitAllowed() throws QInstanceValidationException + { + try + { + qJavalinImplementation.getJavalinMetaData() + .withQueryWithoutLimitAllowed(true); + + QCollectingLogger collectingLogger = QLogger.activateCollectingLoggerForClass(QJavalinImplementation.class); + + String filterJson = """ + {"criteria":[]}"""; + + HttpResponse response = Unirest.post(BASE_URL + "/data/person/query") + .field("filter", filterJson) + .asString(); + + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertTrue(jsonObject.has("records")); + JSONArray records = jsonObject.getJSONArray("records"); + assertEquals(6, records.length()); + + assertThat(collectingLogger.getCollectedMessages()) + .noneMatch(m -> m.getMessage().contains("Query request did not specify a limit")); + } + finally + { + QLogger.activateCollectingLoggerForClass(QJavalinImplementation.class); + resetMetaDataQueryWithoutLimitSettings(); + } + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private void resetMetaDataQueryWithoutLimitSettings() + { + qJavalinImplementation.getJavalinMetaData() + .withQueryWithoutLimitAllowed(false) + .withQueryWithoutLimitDefault(1000) + .withQueryWithoutLimitLogLevel(Level.INFO); + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -1029,7 +1127,7 @@ class QJavalinImplementationTest extends QJavalinTestBase // filter use-case, with no values, should return options. // ///////////////////////////////////////////////////////////// HttpResponse response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=filter").asString(); - JSONArray options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + JSONArray options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); assertNotNull(options); assertThat(options.length()).isGreaterThanOrEqualTo(5); diff --git a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinTestBase.java b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinTestBase.java index f8ee594f..9ca159cd 100644 --- a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinTestBase.java +++ b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinTestBase.java @@ -102,7 +102,7 @@ public class QJavalinTestBase { qJavalinImplementation.stopJavalinServer(); } - qJavalinImplementation = new QJavalinImplementation(qInstance); + qJavalinImplementation = new QJavalinImplementation(qInstance, new QJavalinMetaData()); QJavalinProcessHandler.setAsyncStepTimeoutMillis(250); qJavalinImplementation.startJavalinServer(PORT); } From 3cc0cfd86c21baf4d2ff3a3b7c0d910e7c577038 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 10 Sep 2024 09:34:46 -0500 Subject: [PATCH 03/17] CE-1654 - Just log, don't throw, if missing a security key value (should this be a setting??) --- .../core/actions/audits/AuditAction.java | 8 +++- .../core/actions/audits/AuditActionTest.java | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) 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 518718a9..5cb122f4 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 @@ -225,7 +225,13 @@ public class AuditAction extends AbstractQActionFunction auditRecord = GeneralProcessUtils.getRecordByField("audit", "recordId", recordId); + assertTrue(auditRecord.isPresent()); + + assertThat(collectingLogger.getCollectedMessages()).anyMatch(m -> m.getMessage().contains("Missing securityKeyValue in audit request")); + collectingLogger.clear(); + + //////////////////////////////////////////////////////////////////////////////////////////////// + // try again with a null value in the key - that should be ok - as at least you were thinking // + // about the key and put in SOME value (null has its own semantics in security keys) // + //////////////////////////////////////////////////////////////////////////////////////////////// + Map securityKeys = new HashMap<>(); + securityKeys.put(TestUtils.SECURITY_KEY_TYPE_STORE, null); + AuditAction.execute(TestUtils.TABLE_NAME_ORDER, recordId, securityKeys, "Test Audit"); + + ///////////////////////////////////// + // now the audit should be stored. // + ///////////////////////////////////// + auditRecord = GeneralProcessUtils.getRecordByField("audit", "recordId", recordId); + assertTrue(auditRecord.isPresent()); + assertThat(collectingLogger.getCollectedMessages()).noneMatch(m -> m.getMessage().contains("Missing securityKeyValue in audit request")); + } + + + /******************************************************************************* ** *******************************************************************************/ From 161591405b304f56a7242c3a0f80a1daf3cce77f Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 10 Sep 2024 10:51:21 -0500 Subject: [PATCH 04/17] CE-1654 - do chicken-egg session before the OTHER call to finalizeCustomizeSession too... --- .../Auth0AuthenticationModule.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java index baa1be72..8b871ac1 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/Auth0AuthenticationModule.java @@ -247,10 +247,7 @@ public class Auth0AuthenticationModule implements QAuthenticationModuleInterface ////////////////////////////////////////////////////////////// // allow customizer to do custom things here, if so desired // ////////////////////////////////////////////////////////////// - if(getCustomizer() != null) - { - getCustomizer().finalCustomizeSession(qInstance, qSession); - } + finalCustomizeSession(qInstance, qSession); return (qSession); } @@ -311,14 +308,7 @@ public class Auth0AuthenticationModule implements QAuthenticationModuleInterface ////////////////////////////////////////////////////////////// // allow customizer to do custom things here, if so desired // ////////////////////////////////////////////////////////////// - if(getCustomizer() != null) - { - QContext.withTemporaryContext(QContext.capture(), () -> - { - QContext.setQSession(getChickenAndEggSession()); - getCustomizer().finalCustomizeSession(qInstance, qSession); - }); - } + finalCustomizeSession(qInstance, qSession); return (qSession); } @@ -364,6 +354,23 @@ public class Auth0AuthenticationModule implements QAuthenticationModuleInterface + /*************************************************************************** + ** + ***************************************************************************/ + private void finalCustomizeSession(QInstance qInstance, QSession qSession) + { + if(getCustomizer() != null) + { + QContext.withTemporaryContext(QContext.capture(), () -> + { + QContext.setQSession(getChickenAndEggSession()); + getCustomizer().finalCustomizeSession(qInstance, qSession); + }); + } + } + + + /******************************************************************************* ** Insert a session as a new record into userSession table *******************************************************************************/ From bb548b78d95597f7fbc815f4ef2f6caee90fe838 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 10 Sep 2024 17:28:05 -0500 Subject: [PATCH 05/17] updates to allow override api utils to disable or alter request details --- .../module/api/actions/BaseAPIActionUtil.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 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 54487bd8..209285e0 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 @@ -748,7 +748,7 @@ public class BaseAPIActionUtil { try { - String uri = request.getURI().toString(); + String uri = request.getURI().toString(); String pair = backendMetaData.getApiKeyQueryParamName() + "=" + getApiKey(); /////////////////////////////////////////////////////////////////////////////////// @@ -1167,6 +1167,16 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** + *******************************************************************************/ + protected void logRequestDetails(QTableMetaData table, HttpRequestBase request) throws QException + { + LOG.info("Making [" + request.getMethod() + "] request to URL [" + request.getURI() + "] on table [" + table.getName() + "]."); + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -1192,7 +1202,7 @@ public class BaseAPIActionUtil setupContentTypeInRequest(request); setupAdditionalHeaders(request); - LOG.info("Making [" + request.getMethod() + "] request to URL [" + request.getURI() + "] on table [" + table.getName() + "]."); + logRequestDetails(table, request); if("POST".equals(request.getMethod())) { LOG.info("POST contents [" + ((HttpPost) request).getEntity().toString() + "]"); From c3171c335fabbf45fa40cf031aa859c41a553834 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 17 Sep 2024 16:41:41 -0500 Subject: [PATCH 06/17] Update to always impose a limit on queries (they were getting lost if there was a defaultQueryFilter passed in) --- .../actions/values/SearchPossibleValueSourceAction.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/SearchPossibleValueSourceAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/SearchPossibleValueSourceAction.java index 44b8984a..9cb17a0c 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/SearchPossibleValueSourceAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/values/SearchPossibleValueSourceAction.java @@ -260,9 +260,6 @@ public class SearchPossibleValueSourceAction } } - // todo - skip & limit as params - queryFilter.setLimit(250); - /////////////////////////////////////////////////////////////////////////////////////////////////////// // if given a default filter, make it the 'top level' filter and the one we just created a subfilter // /////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -272,6 +269,9 @@ public class SearchPossibleValueSourceAction queryFilter = input.getDefaultQueryFilter(); } + // todo - skip & limit as params + queryFilter.setLimit(250); + queryFilter.setOrderBys(possibleValueSource.getOrderByFields()); queryInput.setFilter(queryFilter); From febda5123360f21e96a012ed7b75056d9098e5bb Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 26 Sep 2024 15:16:23 -0500 Subject: [PATCH 07/17] CE-1821: added static utility method for returning a list of entities rather than records --- .../core/actions/tables/QueryAction.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/QueryAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/QueryAction.java index af60eeca..c71b57e9 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/QueryAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/QueryAction.java @@ -54,6 +54,7 @@ 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.query.QueryOutput; import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.model.data.QRecordEntity; import com.kingsrook.qqq.backend.core.model.metadata.QBackendMetaData; import com.kingsrook.qqq.backend.core.model.metadata.fields.AdornmentType; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; @@ -266,6 +267,22 @@ public class QueryAction + /******************************************************************************* + ** shorthand way to call for the most common use-case, when you just want the + ** entities to be returned, and you just want to pass in a table name and filter. + *******************************************************************************/ + public static List execute(String tableName, Class entityClass, QQueryFilter filter) throws QException + { + QueryAction queryAction = new QueryAction(); + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(tableName); + queryInput.setFilter(filter); + QueryOutput queryOutput = queryAction.execute(queryInput); + return (queryOutput.getRecordEntities(entityClass)); + } + + + /******************************************************************************* ** shorthand way to call for the most common use-case, when you just want the ** records to be returned, and you just want to pass in a table name and filter. From eb8781db77672fbb6edbc790197b948086b6fd2e Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Wed, 2 Oct 2024 16:16:16 -0500 Subject: [PATCH 08/17] CE-1654 - Update joins built for security-purposes, that if they're for an all-access key, to be outer (LEFT); update tests to reflect this --- .../actions/tables/query/JoinsContext.java | 97 +++++++++++-------- .../rdbms/actions/RDBMSCountActionTest.java | 2 +- .../actions/RDBMSQueryActionJoinsTest.java | 65 ++++++++++++- 3 files changed, 122 insertions(+), 42 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java index 7f2adde6..f1c23ee2 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java @@ -82,7 +82,7 @@ public class JoinsContext ///////////////////////////////////////////////////////////////////////////// // we will get a TON of more output if this gets turned up, so be cautious // ///////////////////////////////////////////////////////////////////////////// - private Level logLevel = Level.OFF; + private Level logLevel = Level.OFF; private Level logLevelForFilter = Level.OFF; @@ -404,6 +404,12 @@ public class JoinsContext chainIsInner = false; } + if(hasAllAccessKey(recordSecurityLock)) + { + queryJoin.withType(QueryJoin.Type.LEFT); + chainIsInner = false; + } + addQueryJoin(queryJoin, "forRecordSecurityLock (non-flipped)", "- "); addedQueryJoins.add(queryJoin); tmpTable = instance.getTable(join.getRightTable()); @@ -423,6 +429,12 @@ public class JoinsContext chainIsInner = false; } + if(hasAllAccessKey(recordSecurityLock)) + { + queryJoin.withType(QueryJoin.Type.LEFT); + chainIsInner = false; + } + addQueryJoin(queryJoin, "forRecordSecurityLock (flipped)", "- "); addedQueryJoins.add(queryJoin); tmpTable = instance.getTable(join.getLeftTable()); @@ -456,44 +468,53 @@ public class JoinsContext + /*************************************************************************** + ** + ***************************************************************************/ + private boolean hasAllAccessKey(RecordSecurityLock recordSecurityLock) + { + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // check if the key type has an all-access key, and if so, if it's set to true for the current user/session // + ////////////////////////////////////////////////////////////////////////////////////////////////////////////// + QSecurityKeyType securityKeyType = instance.getSecurityKeyType(recordSecurityLock.getSecurityKeyType()); + if(StringUtils.hasContent(securityKeyType.getAllAccessKeyName())) + { + QSession session = QContext.getQSession(); + if(session.hasSecurityKeyValue(securityKeyType.getAllAccessKeyName(), true, QFieldType.BOOLEAN)) + { + return (true); + } + } + + return (false); + } + + + /******************************************************************************* ** *******************************************************************************/ private void addSubFilterForRecordSecurityLock(RecordSecurityLock recordSecurityLock, QTableMetaData table, String tableNameOrAlias, boolean isOuter, QueryJoin sourceQueryJoin) { - QSession session = QContext.getQSession(); - - ////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // check if the key type has an all-access key, and if so, if it's set to true for the current user/session // - ////////////////////////////////////////////////////////////////////////////////////////////////////////////// - QSecurityKeyType securityKeyType = instance.getSecurityKeyType(recordSecurityLock.getSecurityKeyType()); - boolean haveAllAccessKey = false; - if(StringUtils.hasContent(securityKeyType.getAllAccessKeyName())) + boolean haveAllAccessKey = hasAllAccessKey(recordSecurityLock); + if(haveAllAccessKey) { - ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // if we have all-access on this key, then we don't need a criterion for it (as long as we're in an AND filter) // - ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - if(session.hasSecurityKeyValue(securityKeyType.getAllAccessKeyName(), true, QFieldType.BOOLEAN)) + if(sourceQueryJoin != null) { - haveAllAccessKey = true; + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // in case the queryJoin object is re-used between queries, and its security criteria need to be different (!!), reset it // + // this can be exposed in tests - maybe not entirely expected in real-world, but seems safe enough // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + sourceQueryJoin.withSecurityCriteria(new ArrayList<>()); + } - if(sourceQueryJoin != null) - { - //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // in case the queryJoin object is re-used between queries, and its security criteria need to be different (!!), reset it // - // this can be exposed in tests - maybe not entirely expected in real-world, but seems safe enough // - //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - sourceQueryJoin.withSecurityCriteria(new ArrayList<>()); - } - - //////////////////////////////////////////////////////////////////////////////////////// - // if we're in an AND filter, then we don't need a criteria for this lock, so return. // - //////////////////////////////////////////////////////////////////////////////////////// - boolean inAnAndFilter = securityFilterCursor.getBooleanOperator() == QQueryFilter.BooleanOperator.AND; - if(inAnAndFilter) - { - return; - } + //////////////////////////////////////////////////////////////////////////////////////// + // if we're in an AND filter, then we don't need a criteria for this lock, so return. // + //////////////////////////////////////////////////////////////////////////////////////// + boolean inAnAndFilter = securityFilterCursor.getBooleanOperator() == QQueryFilter.BooleanOperator.AND; + if(inAnAndFilter) + { + return; } } @@ -545,7 +566,7 @@ public class JoinsContext } else { - List securityKeyValues = session.getSecurityKeyValues(recordSecurityLock.getSecurityKeyType(), type); + List securityKeyValues = QContext.getQSession().getSecurityKeyValues(recordSecurityLock.getSecurityKeyType(), type); if(CollectionUtils.nullSafeIsEmpty(securityKeyValues)) { /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -1193,15 +1214,15 @@ public class JoinsContext if(isStart) { - System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-".repeat(full), full)); + System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-" .repeat(full), full)); } StringBuilder rs = new StringBuilder(); String formatString = "| %-" + md + "s | %-" + md + "s %-" + md + "s | %-" + lg + "s | %-" + sm + "s |\n"; rs.append(String.format(formatString, "Base Table", "Join Table", "(Alias)", "Join Meta Data", "Type")); - String dashesLg = "-".repeat(lg); - String dashesMd = "-".repeat(md); - String dashesSm = "-".repeat(sm); + String dashesLg = "-" .repeat(lg); + String dashesMd = "-" .repeat(md); + String dashesSm = "-" .repeat(sm); rs.append(String.format(formatString, dashesMd, dashesMd, dashesMd, dashesLg, dashesSm)); if(CollectionUtils.nullSafeHasContents(queryJoins)) { @@ -1227,11 +1248,11 @@ public class JoinsContext if(isEnd) { - System.out.println(StringUtils.safeTruncate("--- End " + "-".repeat(full), full) + "\n"); + System.out.println(StringUtils.safeTruncate("--- End " + "-" .repeat(full), full) + "\n"); } else { - System.out.println(StringUtils.safeTruncate("-".repeat(full), full)); + System.out.println(StringUtils.safeTruncate("-" .repeat(full), full)); } } } diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountActionTest.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountActionTest.java index 29247bc7..405c97c8 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountActionTest.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSCountActionTest.java @@ -212,7 +212,7 @@ public class RDBMSCountActionTest extends RDBMSActionTest CountInput countInput = new CountInput(); countInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE); - assertThat(new CountAction().execute(countInput).getCount()).isEqualTo(1); + assertThat(new CountAction().execute(countInput).getCount()).isEqualTo(4); } } diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionJoinsTest.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionJoinsTest.java index ae659653..54e4afcd 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionJoinsTest.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionJoinsTest.java @@ -771,6 +771,61 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest + /******************************************************************************* + ** Error seen in CTLive - query for a record in a sub-table, but whose security + ** key comes from a main table, but the main-table record doesn't exist. + ** + ** In this QInstance, our warehouse table's security key comes from + ** storeWarehouseInt.storeId - so if we insert a warehouse, but no stores, we + ** might not be able to find it (if this bug exists!) + *******************************************************************************/ + @Test + void testRequestedJoinWithTableWhoseSecurityFieldIsInMainTableAndNoRowIsInMainTable() throws Exception + { + runTestSql("INSERT INTO warehouse (name) VALUES ('Springfield')", null); + + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE); + queryInput.setFilter(new QQueryFilter(new QFilterCriteria("name", QCriteriaOperator.EQUALS, "Springfield"))); + + ///////////////////////////////////////// + // with all access key, should find it // + ///////////////////////////////////////// + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1); + + //////////////////////////////////////////// + // with a regular key, should not find it // + //////////////////////////////////////////// + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 1)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(0); + + ///////////////////////////////////////// + // now assign the warehouse to a store // + ///////////////////////////////////////// + runTestSql("INSERT INTO warehouse_store_int (store_id, warehouse_id) SELECT 1, id FROM warehouse WHERE name='Springfield'", null); + + ///////////////////////////////////////// + // with all access key, should find it // + ///////////////////////////////////////// + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1); + + /////////////////////////////////////////////////////// + // with a regular key, should find it if key matches // + /////////////////////////////////////////////////////// + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 1)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(1); + + ////////////////////////////////////////////////////////////////// + // with a regular key, should not find it if key does not match // + ////////////////////////////////////////////////////////////////// + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.TABLE_NAME_STORE, 2)); + assertThat(new QueryAction().execute(queryInput).getRecords()).hasSize(0); + + } + + /******************************************************************************* ** *******************************************************************************/ @@ -888,7 +943,10 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest /******************************************************************************* - ** + ** Note, this test was originally written asserting size=1... but reading + ** the data, for an all-access key, that seems wrong - as the user should see + ** all the records in this table, not just ones associated with a store... + ** so, switching to 4 (same issue in CountActionTest too). *******************************************************************************/ @Test void testRecordSecurityWithLockFromJoinTableWhereTheKeyIsOnTheManySide() throws QException @@ -897,8 +955,9 @@ public class RDBMSQueryActionJoinsTest extends RDBMSActionTest QueryInput queryInput = new QueryInput(); queryInput.setTableName(TestUtils.TABLE_NAME_WAREHOUSE); - assertThat(new QueryAction().execute(queryInput).getRecords()) - .hasSize(1); + List records = new QueryAction().execute(queryInput).getRecords(); + assertThat(records) + .hasSize(4); } From b955a20e1890e0ddc106cf0b4ee26a0f27588cfe Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Wed, 2 Oct 2024 16:22:41 -0500 Subject: [PATCH 09/17] CE-1654 - Checkstyle! --- .../model/actions/tables/query/JoinsContext.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java index f1c23ee2..3f085f0e 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/JoinsContext.java @@ -1214,15 +1214,15 @@ public class JoinsContext if(isStart) { - System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-" .repeat(full), full)); + System.out.println("\n" + StringUtils.safeTruncate("--- Start [main table: " + this.mainTableName + "] " + "-".repeat(full), full)); } StringBuilder rs = new StringBuilder(); String formatString = "| %-" + md + "s | %-" + md + "s %-" + md + "s | %-" + lg + "s | %-" + sm + "s |\n"; rs.append(String.format(formatString, "Base Table", "Join Table", "(Alias)", "Join Meta Data", "Type")); - String dashesLg = "-" .repeat(lg); - String dashesMd = "-" .repeat(md); - String dashesSm = "-" .repeat(sm); + String dashesLg = "-".repeat(lg); + String dashesMd = "-".repeat(md); + String dashesSm = "-".repeat(sm); rs.append(String.format(formatString, dashesMd, dashesMd, dashesMd, dashesLg, dashesSm)); if(CollectionUtils.nullSafeHasContents(queryJoins)) { @@ -1248,11 +1248,11 @@ public class JoinsContext if(isEnd) { - System.out.println(StringUtils.safeTruncate("--- End " + "-" .repeat(full), full) + "\n"); + System.out.println(StringUtils.safeTruncate("--- End " + "-".repeat(full), full) + "\n"); } else { - System.out.println(StringUtils.safeTruncate("-" .repeat(full), full)); + System.out.println(StringUtils.safeTruncate("-".repeat(full), full)); } } } From b687d07e46a7758c44e2b9fe5e6b5b68faf83757 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Fri, 4 Oct 2024 12:24:58 -0500 Subject: [PATCH 10/17] CE-1836: update abstract table sync to make members and functions protected --- .../AbstractTableSyncTransformStep.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 68619a28..3eaba3e2 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -72,33 +72,33 @@ import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; *******************************************************************************/ public abstract class AbstractTableSyncTransformStep extends AbstractTransformStep { - private static final QLogger LOG = QLogger.getLogger(AbstractTableSyncTransformStep.class); + protected static final QLogger LOG = QLogger.getLogger(AbstractTableSyncTransformStep.class); - private ProcessSummaryLine okToInsert = StandardProcessSummaryLineProducer.getOkToInsertLine(); - private ProcessSummaryLine okToUpdate = StandardProcessSummaryLineProducer.getOkToUpdateLine(); + protected ProcessSummaryLine okToInsert = StandardProcessSummaryLineProducer.getOkToInsertLine(); + protected ProcessSummaryLine okToUpdate = StandardProcessSummaryLineProducer.getOkToUpdateLine(); - private ProcessSummaryLine willNotInsert = new ProcessSummaryLine(Status.INFO) + protected ProcessSummaryLine willNotInsert = new ProcessSummaryLine(Status.INFO) .withMessageSuffix("because this process is not configured to insert records.") .withSingularFutureMessage("will not be inserted ") .withPluralFutureMessage("will not be inserted ") .withSingularPastMessage("was not inserted ") .withPluralPastMessage("were not inserted "); - private ProcessSummaryLine willNotUpdate = new ProcessSummaryLine(Status.INFO) + protected ProcessSummaryLine willNotUpdate = new ProcessSummaryLine(Status.INFO) .withMessageSuffix("because this process is not configured to update records.") .withSingularFutureMessage("will not be updated ") .withPluralFutureMessage("will not be updated ") .withSingularPastMessage("was not updated ") .withPluralPastMessage("were not updated "); - private ProcessSummaryLine errorMissingKeyField = new ProcessSummaryLine(Status.ERROR) + protected ProcessSummaryLine errorMissingKeyField = new ProcessSummaryLine(Status.ERROR) .withMessageSuffix("missing a value for the key field.") .withSingularFutureMessage("will not be synced, because it is ") .withPluralFutureMessage("will not be synced, because they are ") .withSingularPastMessage("was not synced, because it is ") .withPluralPastMessage("were not synced, because they are "); - private ProcessSummaryLine unspecifiedError = new ProcessSummaryLine(Status.ERROR) + protected ProcessSummaryLine unspecifiedError = new ProcessSummaryLine(Status.ERROR) .withMessageSuffix("of an unexpected error: ") .withSingularFutureMessage("will not be synced, ") .withPluralFutureMessage("will not be synced, ") @@ -109,7 +109,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt protected RunBackendStepOutput runBackendStepOutput = null; protected RecordLookupHelper recordLookupHelper = null; - private QPossibleValueTranslator possibleValueTranslator; + protected QPossibleValueTranslator possibleValueTranslator; @@ -374,6 +374,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt } + /******************************************************************************* ** Given a source record, extract what we'll use as its key from it. ** From 4f92fb2ae230d8879d11ec0eb24e3e01092e4201 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Mon, 7 Oct 2024 22:33:16 -0500 Subject: [PATCH 11/17] CE-1836: updates to allow getting basepull key value and sync config perform insert/updates from input --- .../actions/processes/RunProcessAction.java | 11 ++++-- .../AbstractTableSyncTransformStep.java | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/processes/RunProcessAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/processes/RunProcessAction.java index 03ee26aa..ada00f9a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/processes/RunProcessAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/processes/RunProcessAction.java @@ -79,6 +79,7 @@ public class RunProcessAction { private static final QLogger LOG = QLogger.getLogger(RunProcessAction.class); + public static final String BASEPULL_KEY_VALUE = "basepullKeyValue"; public static final String BASEPULL_THIS_RUNTIME_KEY = "basepullThisRuntimeKey"; public static final String BASEPULL_LAST_RUNTIME_KEY = "basepullLastRuntimeKey"; public static final String BASEPULL_TIMESTAMP_FIELD = "basepullTimestampField"; @@ -517,9 +518,13 @@ public class RunProcessAction /******************************************************************************* ** *******************************************************************************/ - protected String determineBasepullKeyValue(QProcessMetaData process, BasepullConfiguration basepullConfiguration) throws QException + protected String determineBasepullKeyValue(QProcessMetaData process, RunProcessInput runProcessInput, BasepullConfiguration basepullConfiguration) throws QException { String basepullKeyValue = (basepullConfiguration.getKeyValue() != null) ? basepullConfiguration.getKeyValue() : process.getName(); + if(runProcessInput.getValueString(BASEPULL_KEY_VALUE) != null) + { + basepullKeyValue = runProcessInput.getValueString(BASEPULL_KEY_VALUE); + } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // if process specifies that it uses variants, look for that data in the session and append to our basepull key // @@ -551,7 +556,7 @@ public class RunProcessAction String basepullTableName = basepullConfiguration.getTableName(); String basepullKeyFieldName = basepullConfiguration.getKeyField(); String basepullLastRunTimeFieldName = basepullConfiguration.getLastRunTimeFieldName(); - String basepullKeyValue = determineBasepullKeyValue(process, basepullConfiguration); + String basepullKeyValue = determineBasepullKeyValue(process, runProcessInput, basepullConfiguration); /////////////////////////////////////// // get the stored basepull timestamp // @@ -631,7 +636,7 @@ public class RunProcessAction String basepullKeyFieldName = basepullConfiguration.getKeyField(); String basepullLastRunTimeFieldName = basepullConfiguration.getLastRunTimeFieldName(); Integer basepullHoursBackForInitialTimestamp = basepullConfiguration.getHoursBackForInitialTimestamp(); - String basepullKeyValue = determineBasepullKeyValue(process, basepullConfiguration); + String basepullKeyValue = determineBasepullKeyValue(process, runProcessInput, basepullConfiguration); /////////////////////////////////////// // get the stored basepull timestamp // diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 3eaba3e2..fed50bad 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -111,6 +111,9 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt protected QPossibleValueTranslator possibleValueTranslator; + protected static final String SYNC_TABLE_PERFORM_INSERTS_KEY = "syncTablePerformInsertsKey"; + protected static final String SYNC_TABLE_PERFORM_UPDATES_KEY = "syncTablePerformUpdatesKey"; + /******************************************************************************* @@ -193,6 +196,26 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt + /******************************************************************************* + ** + *******************************************************************************/ + void setPerformInserts(boolean performInserts) + { + this.setPerformInserts(performInserts); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + void setPerformUpdates(boolean performUpdates) + { + this.setPerformUpdates(performUpdates); + } + + + /******************************************************************************* ** artificial method, here to make jacoco see that this class is indeed ** included in test coverage... @@ -222,6 +245,18 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt SyncProcessConfig config = getSyncProcessConfig(); + //////////////////////////////////////////////////////////// + // see if these fields have been updated via input fields // + //////////////////////////////////////////////////////////// + if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY) != null) + { + config.setPerformInserts(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY))); + } + if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY) != null) + { + config.setPerformUpdates(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY))); + } + String sourceTableKeyField = config.sourceTableKeyField; String destinationTableForeignKeyField = config.destinationTableForeignKey; String destinationTableName = config.destinationTable; From 526ba6ca307c727891758eb96aae9cd53acc6467 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 8 Oct 2024 15:46:47 -0500 Subject: [PATCH 12/17] CE-1836: added potential to log output --- .../AbstractTableSyncTransformStep.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index fed50bad..920d5090 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -23,6 +23,8 @@ package com.kingsrook.qqq.backend.core.processes.implementations.tablesync; import java.io.Serializable; +import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -35,6 +37,7 @@ import java.util.Set; import java.util.stream.Collectors; import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.actions.values.QPossibleValueTranslator; +import com.kingsrook.qqq.backend.core.actions.values.QValueFormatter; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.logging.QLogger; @@ -53,6 +56,7 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryJoin; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryOutput; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; +import com.kingsrook.qqq.backend.core.model.session.QSession; import com.kingsrook.qqq.backend.core.processes.implementations.etl.streamedwithfrontend.AbstractTransformStep; import com.kingsrook.qqq.backend.core.processes.implementations.etl.streamedwithfrontend.StreamedETLWithFrontendProcess; import com.kingsrook.qqq.backend.core.processes.implementations.general.StandardProcessSummaryLineProducer; @@ -113,6 +117,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt protected static final String SYNC_TABLE_PERFORM_INSERTS_KEY = "syncTablePerformInsertsKey"; protected static final String SYNC_TABLE_PERFORM_UPDATES_KEY = "syncTablePerformUpdatesKey"; + protected static final String LOG_TRANSFORM_RESULTS = "logTransformResults"; @@ -237,6 +242,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt { if(CollectionUtils.nullSafeIsEmpty(runBackendStepInput.getRecords())) { + LOG.info("No input records were found."); return; } @@ -406,6 +412,53 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt possibleValueTranslator.translatePossibleValuesInRecords(QContext.getQInstance().getTable(destinationTableName), runBackendStepOutput.getRecords()); } } + + if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY) != null) + { + logResults(runBackendStepInput, config); + } + } + + + + /******************************************************************************* + ** Log results of transformation + ** + *******************************************************************************/ + protected void logResults(RunBackendStepInput runBackendStepInput, SyncProcessConfig syncProcessConfig) + { + String timezone = QContext.getQSession().getValue(QSession.VALUE_KEY_USER_TIMEZONE); + if(timezone == null) + { + timezone = QContext.getQInstance().getDefaultTimeZoneId(); + } + ZonedDateTime dateTime = runBackendStepInput.getBasepullLastRunTime().atZone(ZoneId.of(timezone)); + + if(syncProcessConfig.performInserts) + { + if(okToInsert.getCount() == 0) + { + LOG.info("No Records were found to insert since " + QValueFormatter.formatDateTimeWithZone(dateTime) + "."); + } + else + { + String pluralized = okToInsert.getCount() > 1 ? " Records were " : " Record was "; + LOG.info(okToInsert.getCount() + pluralized + " found to insert since " + QValueFormatter.formatDateTimeWithZone(dateTime) + ".", logPair("primaryKeys", okToInsert.getPrimaryKeys())); + } + } + + if(syncProcessConfig.performUpdates) + { + if(okToUpdate.getCount() == 0) + { + LOG.info("No Records were found to update since " + QValueFormatter.formatDateTimeWithZone(dateTime) + "."); + } + else + { + String pluralized = okToUpdate.getCount() > 1 ? " Records were " : " Record was "; + LOG.info(okToUpdate.getCount() + pluralized + " found to update since " + QValueFormatter.formatDateTimeWithZone(dateTime) + ".", logPair("primaryKeys", okToInsert.getPrimaryKeys())); + } + } } From 10014f16ae42e945703dd8a35403aa02d8d39630 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Tue, 8 Oct 2024 16:20:23 -0500 Subject: [PATCH 13/17] CE-1836: fixed to check as boolean --- .../tablesync/AbstractTableSyncTransformStep.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 920d5090..7168321a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -262,7 +262,6 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt { config.setPerformUpdates(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY))); } - String sourceTableKeyField = config.sourceTableKeyField; String destinationTableForeignKeyField = config.destinationTableForeignKey; String destinationTableName = config.destinationTable; @@ -413,7 +412,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt } } - if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY) != null) + if(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY))) { logResults(runBackendStepInput, config); } From e0597827ef8b4d4c83c488651a155df0df673624 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Wed, 9 Oct 2024 10:30:49 -0500 Subject: [PATCH 14/17] CE-1836: updates from code review --- .../AbstractTableSyncTransformStep.java | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 7168321a..9b0934a0 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -201,26 +201,6 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt - /******************************************************************************* - ** - *******************************************************************************/ - void setPerformInserts(boolean performInserts) - { - this.setPerformInserts(performInserts); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - void setPerformUpdates(boolean performUpdates) - { - this.setPerformUpdates(performUpdates); - } - - - /******************************************************************************* ** artificial method, here to make jacoco see that this class is indeed ** included in test coverage... @@ -256,11 +236,11 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt //////////////////////////////////////////////////////////// if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY) != null) { - config.setPerformInserts(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY))); + config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, true, config.performUpdates); } if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY) != null) { - config.setPerformUpdates(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY))); + config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, config.performUpdates, true); } String sourceTableKeyField = config.sourceTableKeyField; String destinationTableForeignKeyField = config.destinationTableForeignKey; @@ -412,7 +392,7 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt } } - if(Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY))) + if(Boolean.parseBoolean(runBackendStepInput.getValueString(LOG_TRANSFORM_RESULTS))) { logResults(runBackendStepInput, config); } From 766881dee044a4b56b1d657549e7798c2b72fd3a Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 10 Oct 2024 09:59:20 -0500 Subject: [PATCH 15/17] CE-1836: fixed npe if last basepull runtime hadnt been set --- .../tablesync/AbstractTableSyncTransformStep.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 9b0934a0..71f80d86 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -23,6 +23,7 @@ package com.kingsrook.qqq.backend.core.processes.implementations.tablesync; import java.io.Serializable; +import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.ArrayList; @@ -411,7 +412,13 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt { timezone = QContext.getQInstance().getDefaultTimeZoneId(); } - ZonedDateTime dateTime = runBackendStepInput.getBasepullLastRunTime().atZone(ZoneId.of(timezone)); + Instant lastRunTime = Instant.now(); + if(runBackendStepInput.getBasepullLastRunTime() != null) + { + lastRunTime = runBackendStepInput.getBasepullLastRunTime(); + } + + ZonedDateTime dateTime = lastRunTime.atZone(ZoneId.of(timezone)); if(syncProcessConfig.performInserts) { From ff1e022798d6beae71aaa8d5ecaf802ea262a7e1 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 10 Oct 2024 11:31:43 -0500 Subject: [PATCH 16/17] CE-1836: wasn't properly using boolean values in backend step input --- .../tablesync/AbstractTableSyncTransformStep.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java index 71f80d86..ca998f4f 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/tablesync/AbstractTableSyncTransformStep.java @@ -237,11 +237,13 @@ public abstract class AbstractTableSyncTransformStep extends AbstractTransformSt //////////////////////////////////////////////////////////// if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY) != null) { - config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, true, config.performUpdates); + boolean performInserts = Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_INSERTS_KEY)); + config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, performInserts, config.performUpdates); } if(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY) != null) { - config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, config.performUpdates, true); + boolean performUpdates = Boolean.parseBoolean(runBackendStepInput.getValueString(SYNC_TABLE_PERFORM_UPDATES_KEY)); + config = new SyncProcessConfig(config.sourceTable, config.sourceTableKeyField, config.destinationTable, config.destinationTableForeignKey, config.performUpdates, performUpdates); } String sourceTableKeyField = config.sourceTableKeyField; String destinationTableForeignKeyField = config.destinationTableForeignKey; From bbf4c2c2ff8f810be6db7d88ec2d651255a5937a Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 10 Oct 2024 18:12:45 -0500 Subject: [PATCH 17/17] hotfix: allow basepull override values from jobs --- .../basepull/ExtractViaBasepullQueryStep.java | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/basepull/ExtractViaBasepullQueryStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/basepull/ExtractViaBasepullQueryStep.java index 88e3352c..470ab381 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/basepull/ExtractViaBasepullQueryStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/basepull/ExtractViaBasepullQueryStep.java @@ -40,6 +40,10 @@ import com.kingsrook.qqq.backend.core.processes.implementations.etl.streamedwith *******************************************************************************/ public class ExtractViaBasepullQueryStep extends ExtractViaQueryStep implements BasepullExtractStepInterface { + protected static final String SECONDS_TO_SUBTRACT_FROM_THIS_RUN_TIME_KEY = "secondsToSubtractFromThisRunTimeForTimestampQuery"; + protected static final String SECONDS_TO_SUBTRACT_FROM_LAST_RUN_TIME_KEY = "secondsToSubtractFromLastRunTimeForTimestampQuery"; + + /******************************************************************************* ** @@ -124,7 +128,8 @@ public class ExtractViaBasepullQueryStep extends ExtractViaQueryStep implements *******************************************************************************/ protected String getLastRunTimeString(RunBackendStepInput runBackendStepInput) throws QException { - Instant lastRunTime = runBackendStepInput.getBasepullLastRunTime(); + Instant lastRunTime = runBackendStepInput.getBasepullLastRunTime(); + Instant updatedRunTime = lastRunTime; ////////////////////////////////////////////////////////////////////////////////////////////// // allow the timestamps to be adjusted by the specified number of seconds. // @@ -135,10 +140,19 @@ public class ExtractViaBasepullQueryStep extends ExtractViaQueryStep implements Serializable basepullConfigurationValue = runBackendStepInput.getValue(RunProcessAction.BASEPULL_CONFIGURATION); if(basepullConfigurationValue instanceof BasepullConfiguration basepullConfiguration && basepullConfiguration.getSecondsToSubtractFromLastRunTimeForTimestampQuery() != null) { - lastRunTime = lastRunTime.minusSeconds(basepullConfiguration.getSecondsToSubtractFromLastRunTimeForTimestampQuery()); + updatedRunTime = lastRunTime.minusSeconds(basepullConfiguration.getSecondsToSubtractFromLastRunTimeForTimestampQuery()); } - return (lastRunTime.toString()); + ////////////////////////////////////////////////////////////// + // if an override was found in the params, use that instead // + ////////////////////////////////////////////////////////////// + if(runBackendStepInput.getValueString(SECONDS_TO_SUBTRACT_FROM_LAST_RUN_TIME_KEY) != null) + { + int secondsBack = Integer.parseInt(runBackendStepInput.getValueString(SECONDS_TO_SUBTRACT_FROM_LAST_RUN_TIME_KEY)); + updatedRunTime = lastRunTime.minusSeconds(secondsBack); + } + + return (updatedRunTime.toString()); } @@ -148,14 +162,24 @@ public class ExtractViaBasepullQueryStep extends ExtractViaQueryStep implements *******************************************************************************/ protected String getThisRunTimeString(RunBackendStepInput runBackendStepInput) throws QException { - Instant thisRunTime = runBackendStepInput.getValueInstant(RunProcessAction.BASEPULL_THIS_RUNTIME_KEY); + Instant thisRunTime = runBackendStepInput.getValueInstant(RunProcessAction.BASEPULL_THIS_RUNTIME_KEY); + Instant updatedRunTime = thisRunTime; Serializable basepullConfigurationValue = runBackendStepInput.getValue(RunProcessAction.BASEPULL_CONFIGURATION); if(basepullConfigurationValue instanceof BasepullConfiguration basepullConfiguration && basepullConfiguration.getSecondsToSubtractFromThisRunTimeForTimestampQuery() != null) { - thisRunTime = thisRunTime.minusSeconds(basepullConfiguration.getSecondsToSubtractFromThisRunTimeForTimestampQuery()); + updatedRunTime = thisRunTime.minusSeconds(basepullConfiguration.getSecondsToSubtractFromThisRunTimeForTimestampQuery()); } - return (thisRunTime.toString()); + ////////////////////////////////////////////////////////////// + // if an override was found in the params, use that instead // + ////////////////////////////////////////////////////////////// + if(runBackendStepInput.getValueString(SECONDS_TO_SUBTRACT_FROM_THIS_RUN_TIME_KEY) != null) + { + int secondsBack = Integer.parseInt(runBackendStepInput.getValueString(SECONDS_TO_SUBTRACT_FROM_THIS_RUN_TIME_KEY)); + updatedRunTime = thisRunTime.minusSeconds(secondsBack); + } + + return (updatedRunTime.toString()); } }