From 3398b812cedad70751fc7e8bd4feda42a9c8ff07 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 27 Jun 2024 11:50:37 -0500 Subject: [PATCH 1/4] Add logging at various increasing levels if more and more records get added to a QueryOutputList --- .../actions/tables/query/QueryOutput.java | 2 +- .../actions/tables/query/QueryOutputList.java | 78 ++++++++++++- .../tables/query/QueryOutputListTest.java | 110 ++++++++++++++++++ 3 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputListTest.java diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutput.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutput.java index bbee71d3..1f2ae7d9 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutput.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutput.java @@ -53,7 +53,7 @@ public class QueryOutput extends AbstractActionOutput implements Serializable } else { - storage = new QueryOutputList(); + storage = new QueryOutputList(queryInput); } } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputList.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputList.java index 9d7ee0f4..e9eaac70 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputList.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputList.java @@ -24,7 +24,10 @@ package com.kingsrook.qqq.backend.core.model.actions.tables.query; import java.util.ArrayList; import java.util.List; +import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.data.QRecord; +import org.apache.logging.log4j.Level; +import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; /******************************************************************************* @@ -33,15 +36,50 @@ import com.kingsrook.qqq.backend.core.model.data.QRecord; *******************************************************************************/ class QueryOutputList implements QueryOutputStorageInterface { - private List records = new ArrayList<>(); + private static final QLogger LOG = QLogger.getLogger(QueryOutputList.class); + + private final String tableName; + private List records = new ArrayList<>(); + + private static int LOG_SIZE_INFO_OVER = 50_000; + private static int LOG_SIZE_WARN_OVER = 100_000; + private static int LOG_SIZE_ERROR_OVER = 250_000; /******************************************************************************* ** *******************************************************************************/ - public QueryOutputList() + public QueryOutputList(QueryInput queryInput) { + tableName = queryInput.getTableName(); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private void logSize(int sizeBefore, int sizeAfter) + { + Level level = null; + if(sizeBefore < LOG_SIZE_ERROR_OVER && sizeAfter >= LOG_SIZE_ERROR_OVER) + { + level = Level.ERROR; + } + else if(sizeBefore < LOG_SIZE_WARN_OVER && sizeAfter >= LOG_SIZE_WARN_OVER) + { + level = Level.WARN; + } + else if(sizeBefore < LOG_SIZE_INFO_OVER && sizeAfter >= LOG_SIZE_INFO_OVER) + { + level = Level.INFO; + } + + if(level != null) + { + LOG.log(level, "Large number of records in QueryOutputList", new Throwable(), logPair("noRecords", sizeAfter), logPair("tableName", tableName)); + } } @@ -52,7 +90,9 @@ class QueryOutputList implements QueryOutputStorageInterface @Override public void addRecord(QRecord record) { + int sizeBefore = this.records.size(); records.add(record); + logSize(sizeBefore, this.records.size()); } @@ -63,7 +103,9 @@ class QueryOutputList implements QueryOutputStorageInterface @Override public void addRecords(List records) { + int sizeBefore = this.records.size(); this.records.addAll(records); + logSize(sizeBefore, this.records.size()); } @@ -77,4 +119,36 @@ class QueryOutputList implements QueryOutputStorageInterface return (records); } + + + /******************************************************************************* + ** Setter for LOG_SIZE_INFO_OVER + ** + *******************************************************************************/ + public static void setLogSizeInfoOver(int logSizeInfoOver) + { + QueryOutputList.LOG_SIZE_INFO_OVER = logSizeInfoOver; + } + + + + /******************************************************************************* + ** Setter for LOG_SIZE_WARN_OVER + ** + *******************************************************************************/ + public static void setLogSizeWarnOver(int logSizeWarnOver) + { + QueryOutputList.LOG_SIZE_WARN_OVER = logSizeWarnOver; + } + + + + /******************************************************************************* + ** Setter for LOG_SIZE_ERROR_OVER + ** + *******************************************************************************/ + public static void setLogSizeErrorOver(int logSizeErrorOver) + { + QueryOutputList.LOG_SIZE_ERROR_OVER = logSizeErrorOver; + } } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputListTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputListTest.java new file mode 100644 index 00000000..4d2c68b6 --- /dev/null +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QueryOutputListTest.java @@ -0,0 +1,110 @@ +/* + * 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.model.actions.tables.query; + + +import java.util.Collections; +import com.kingsrook.qqq.backend.core.BaseTest; +import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.logging.QCollectingLogger; +import com.kingsrook.qqq.backend.core.logging.QLogger; +import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.utils.TestUtils; +import org.apache.logging.log4j.Level; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + + +/******************************************************************************* + ** Unit test for QueryOutputList + *******************************************************************************/ +class QueryOutputListTest extends BaseTest +{ + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testLogSize() throws QException + { + QueryInput queryInput = new QueryInput(TestUtils.TABLE_NAME_PERSON); + QueryOutput queryOutput = new QueryOutput(queryInput); + + QCollectingLogger collectingLogger = QLogger.activateCollectingLoggerForClass(QueryOutputList.class); + + /////////////////////// + // set up our limits // + /////////////////////// + int infoLimit = 10; + int warnLimit = 20; + int errorLimit = 30; + + QueryOutputList.setLogSizeInfoOver(infoLimit); + QueryOutputList.setLogSizeWarnOver(warnLimit); + QueryOutputList.setLogSizeErrorOver(errorLimit); + + //////////////////////////// + // add records one-by-one // + //////////////////////////// + for(int i = 0; i < errorLimit; i++) + { + queryOutput.addRecord(new QRecord()); + } + + /////////////////////////////////////////////////////////////// + // assert we got the expected logs as each level was crossed // + /////////////////////////////////////////////////////////////// + assertEquals(3, collectingLogger.getCollectedMessages().size()); + + assertEquals(Level.INFO, collectingLogger.getCollectedMessages().get(0).getLevel()); + assertThat(collectingLogger.getCollectedMessages().get(0).getMessage()) + .contains("\"noRecords\":" + infoLimit) + .contains("\"tableName\":\"" + TestUtils.TABLE_NAME_PERSON + "\""); + + assertEquals(Level.WARN, collectingLogger.getCollectedMessages().get(1).getLevel()); + assertThat(collectingLogger.getCollectedMessages().get(1).getMessage()) + .contains("\"noRecords\":" + warnLimit) + .contains("\"tableName\":\"" + TestUtils.TABLE_NAME_PERSON + "\""); + + assertEquals(Level.ERROR, collectingLogger.getCollectedMessages().get(2).getLevel()); + assertThat(collectingLogger.getCollectedMessages().get(2).getMessage()) + .contains("\"noRecords\":" + errorLimit) + .contains("\"tableName\":\"" + TestUtils.TABLE_NAME_PERSON + "\""); + + ////////////////////////////////////////////////////////////////////////////////////////// + // reset the logger - then run again, doing a bulk add that goes straight to error size // + ////////////////////////////////////////////////////////////////////////////////////////// + collectingLogger.clear(); + queryOutput = new QueryOutput(queryInput); + int bulkSize = errorLimit + 1; + queryOutput.addRecords(Collections.nCopies(bulkSize, new QRecord())); + + assertEquals(1, collectingLogger.getCollectedMessages().size()); + assertEquals(Level.ERROR, collectingLogger.getCollectedMessages().get(0).getLevel()); + assertThat(collectingLogger.getCollectedMessages().get(0).getMessage()) + .contains("\"noRecords\":" + bulkSize) + .contains("\"tableName\":\"" + TestUtils.TABLE_NAME_PERSON + "\""); + + } + +} \ No newline at end of file From a4295df20de5962d9ac0200a3161c7ec32a0477c Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 27 Jun 2024 11:50:58 -0500 Subject: [PATCH 2/4] Mark getInstance and getSession as deprecated - they already just return value from QContext, but callers should instead do that directly. --- .../qqq/backend/core/model/actions/AbstractActionInput.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/AbstractActionInput.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/AbstractActionInput.java index 3c98715b..cee3d521 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/AbstractActionInput.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/AbstractActionInput.java @@ -107,8 +107,10 @@ public class AbstractActionInput /******************************************************************************* ** Getter for instance ** + ** Deprecated. Please use QContext.getInstance() instead *******************************************************************************/ @JsonIgnore + @Deprecated public QInstance getInstance() { return (QContext.getQInstance()); @@ -119,8 +121,10 @@ public class AbstractActionInput /******************************************************************************* ** Getter for session ** + ** Deprecated. Please use QContext.getSession() instead *******************************************************************************/ @JsonIgnore + @Deprecated public QSession getSession() { return (QContext.getQSession()); From b24a99004366520ad82e6064fc9269ae209ac245 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 27 Jun 2024 11:51:10 -0500 Subject: [PATCH 3/4] Make end-of-job log message use log pairs --- .../qqq/backend/core/actions/async/AsyncRecordPipeLoop.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/async/AsyncRecordPipeLoop.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/async/AsyncRecordPipeLoop.java index 74430f7f..cbcb6397 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/async/AsyncRecordPipeLoop.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/async/AsyncRecordPipeLoop.java @@ -32,6 +32,7 @@ import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.utils.SleepUtils; import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeFunction; import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeSupplier; +import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; /******************************************************************************* @@ -185,8 +186,7 @@ public class AsyncRecordPipeLoop if(recordCount > 0) { - LOG.info(String.format("Processed %,d records", recordCount) - + String.format(" at end of job [%s] in %,d ms (%.2f records/second).", jobName, (endTime - jobStartTime), 1000d * (recordCount / (.001d + (endTime - jobStartTime))))); + LOG.info("End of job summary", logPair("recordCount", recordCount), logPair("jobName", jobName), logPair("millis", endTime - jobStartTime), logPair("recordsPerSecond", 1000d * (recordCount / (.001d + (endTime - jobStartTime))))); } return (recordCount); From 18c94943cb25f0263c81263c34e9d88ef11a2cbe Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 27 Jun 2024 11:52:51 -0500 Subject: [PATCH 4/4] Enrich apps before tables - fixed a situation where a table's possible-value field wasn't getting set up as LINK adornment, due to table not being put in app's child-list, which enrichment does, so, if we enrich app first, it fixed it --- .../core/instances/QInstanceEnricher.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceEnricher.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceEnricher.java index a87713e5..0e101379 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceEnricher.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceEnricher.java @@ -123,6 +123,18 @@ public class QInstanceEnricher *******************************************************************************/ public void enrich() { + ///////////////////////////////////////////////////////////////////////////////////////// + // at one point, we did apps later - but - it was possible to put tables in an app's // + // sections, but not its children list (enrichApp fixes this by adding such tables to // + // the children list) so then when enrichTable runs, it looks for fields that are // + // possible-values pointed at tables, for adding LINK adornments - and that could // + // cause such links to be omitted, mysteriously! so, do app enrichment before tables. // + ///////////////////////////////////////////////////////////////////////////////////////// + if(qInstance.getApps() != null) + { + qInstance.getApps().values().forEach(this::enrichApp); + } + if(qInstance.getTables() != null) { qInstance.getTables().values().forEach(this::enrichTable); @@ -139,11 +151,6 @@ public class QInstanceEnricher qInstance.getBackends().values().forEach(this::enrichBackend); } - if(qInstance.getApps() != null) - { - qInstance.getApps().values().forEach(this::enrichApp); - } - if(qInstance.getReports() != null) { qInstance.getReports().values().forEach(this::enrichReport);