From 4cbd808a558abd56fff71c8dfecc64ca99c645bc Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 4 Aug 2023 19:39:06 -0500 Subject: [PATCH 01/13] CE-607 add query joins to GetInput --- .../tables/QueryOrGetInputInterface.java | 16 ++++++ .../model/actions/tables/get/GetInput.java | 51 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/QueryOrGetInputInterface.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/QueryOrGetInputInterface.java index 63f175e9..42804602 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/QueryOrGetInputInterface.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/QueryOrGetInputInterface.java @@ -23,7 +23,9 @@ package com.kingsrook.qqq.backend.core.model.actions.tables; import java.util.Collection; +import java.util.List; import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryJoin; /******************************************************************************* @@ -49,6 +51,7 @@ public interface QueryOrGetInputInterface this.setShouldMaskPasswords(source.getShouldMaskPasswords()); this.setIncludeAssociations(source.getIncludeAssociations()); this.setAssociationNamesToInclude(source.getAssociationNamesToInclude()); + this.setQueryJoins(source.getQueryJoins()); } /******************************************************************************* @@ -146,4 +149,17 @@ public interface QueryOrGetInputInterface *******************************************************************************/ void setAssociationNamesToInclude(Collection associationNamesToInclude); + + /******************************************************************************* + ** Getter for queryJoins + *******************************************************************************/ + List getQueryJoins(); + + + /******************************************************************************* + ** Setter for queryJoins + ** + *******************************************************************************/ + void setQueryJoins(List queryJoins); + } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/get/GetInput.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/get/GetInput.java index 2b7f8ba6..2cf7872f 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/get/GetInput.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/get/GetInput.java @@ -23,11 +23,14 @@ package com.kingsrook.qqq.backend.core.model.actions.tables.get; import java.io.Serializable; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Map; import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; import com.kingsrook.qqq.backend.core.model.actions.AbstractTableActionInput; import com.kingsrook.qqq.backend.core.model.actions.tables.QueryOrGetInputInterface; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryJoin; /******************************************************************************* @@ -47,6 +50,7 @@ public class GetInput extends AbstractTableActionInput implements QueryOrGetInpu private boolean shouldOmitHiddenFields = true; private boolean shouldMaskPasswords = true; + private List queryJoins = null; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // if you say you want to includeAssociations, you can limit which ones by passing them in associationNamesToInclude. // @@ -411,4 +415,51 @@ public class GetInput extends AbstractTableActionInput implements QueryOrGetInpu return (this); } + + + /******************************************************************************* + ** Getter for queryJoins + *******************************************************************************/ + public List getQueryJoins() + { + return (this.queryJoins); + } + + + + /******************************************************************************* + ** Setter for queryJoins + *******************************************************************************/ + public void setQueryJoins(List queryJoins) + { + this.queryJoins = queryJoins; + } + + + + /******************************************************************************* + ** Fluent setter for queryJoins + *******************************************************************************/ + public GetInput withQueryJoins(List queryJoins) + { + this.queryJoins = queryJoins; + return (this); + } + + + + /******************************************************************************* + ** Fluent setter for queryJoins + ** + *******************************************************************************/ + public GetInput withQueryJoin(QueryJoin queryJoin) + { + if(this.queryJoins == null) + { + this.queryJoins = new ArrayList<>(); + } + this.queryJoins.add(queryJoin); + return (this); + } + } From 4cb00670ed149b614544000073f7f5df040e0cb3 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 4 Aug 2023 19:39:28 -0500 Subject: [PATCH 02/13] CE-607 Switch a tryElse to a tryAndRequireNonNullElse, to avoid NPE --- .../java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java index beb902d5..8b16305a 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java @@ -157,7 +157,7 @@ public class QRecordApiAdapter *******************************************************************************/ private static boolean isAssociationOmitted(String apiName, String apiVersion, QTableMetaData table, Association association) { - ApiTableMetaData thisApiTableMetaData = ObjectUtils.tryElse(() -> ApiTableMetaDataContainer.of(table).getApiTableMetaData(apiName), new ApiTableMetaData()); + ApiTableMetaData thisApiTableMetaData = ObjectUtils.tryAndRequireNonNullElse(() -> ApiTableMetaDataContainer.of(table).getApiTableMetaData(apiName), new ApiTableMetaData()); ApiAssociationMetaData apiAssociationMetaData = thisApiTableMetaData.getApiAssociationMetaData().get(association.getName()); if(apiAssociationMetaData != null) { From d811ed725d59c1585f78bc3b2e843a4ada0b48a9 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 13:17:11 -0500 Subject: [PATCH 03/13] Support api queryCriteria and orderBy for removed fields; more/better use of api names for tables & fields in openApi spec; pass qInstance through supplemental validation chain; --- .../core/instances/QInstanceEnricher.java | 2 +- .../tables/QSupplementalTableMetaData.java | 5 +- .../qqq/api/actions/ApiImplementation.java | 69 ++++++++++++++----- .../actions/GenerateOpenApiSpecAction.java | 38 ++++++---- .../api/actions/GetTableApiFieldsAction.java | 62 +++++++++++++++++ .../qqq/api/actions/QRecordApiAdapter.java | 69 ++----------------- .../actions/ApiFieldCustomValueMapper.java | 39 ++++++++++- .../metadata/tables/ApiTableMetaData.java | 5 +- .../tables/ApiTableMetaDataContainer.java | 7 +- .../api/actions/ApiImplementationTest.java | 52 +++++++++++++- 10 files changed, 238 insertions(+), 110 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 a8afc8c6..84016c21 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 @@ -272,7 +272,7 @@ public class QInstanceEnricher for(QSupplementalTableMetaData supplementalTableMetaData : CollectionUtils.nonNullMap(table.getSupplementalMetaData()).values()) { - supplementalTableMetaData.enrich(table); + supplementalTableMetaData.enrich(qInstance, table); } } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/QSupplementalTableMetaData.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/QSupplementalTableMetaData.java index d4bc6baf..d0dc48e4 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/QSupplementalTableMetaData.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/QSupplementalTableMetaData.java @@ -22,6 +22,9 @@ package com.kingsrook.qqq.backend.core.model.metadata.tables; +import com.kingsrook.qqq.backend.core.model.metadata.QInstance; + + /******************************************************************************* ** Base-class for table-level meta-data defined by some supplemental module, etc, ** outside of qqq core @@ -60,7 +63,7 @@ public abstract class QSupplementalTableMetaData /******************************************************************************* ** *******************************************************************************/ - public void enrich(QTableMetaData table) + public void enrich(QInstance qInstance, QTableMetaData table) { //////////////////////// // noop in base class // diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/ApiImplementation.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/ApiImplementation.java index 9abe151e..842ab777 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/ApiImplementation.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/ApiImplementation.java @@ -36,9 +36,12 @@ import java.util.UUID; import java.util.concurrent.TimeUnit; import com.kingsrook.qqq.api.javalin.QBadRequestException; import com.kingsrook.qqq.api.model.APIVersion; +import com.kingsrook.qqq.api.model.actions.ApiFieldCustomValueMapper; import com.kingsrook.qqq.api.model.actions.HttpApiResponse; import com.kingsrook.qqq.api.model.metadata.ApiInstanceMetaData; import com.kingsrook.qqq.api.model.metadata.ApiOperation; +import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaData; +import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaDataContainer; import com.kingsrook.qqq.api.model.metadata.processes.ApiProcessCustomizers; import com.kingsrook.qqq.api.model.metadata.processes.ApiProcessInput; import com.kingsrook.qqq.api.model.metadata.processes.ApiProcessInputFieldsContainer; @@ -104,6 +107,7 @@ import com.kingsrook.qqq.backend.core.model.statusmessages.QWarningMessage; import com.kingsrook.qqq.backend.core.processes.implementations.etl.streamedwithfrontend.CouldNotFindQueryFilterForExtractStepException; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.ExceptionUtils; +import com.kingsrook.qqq.backend.core.utils.ObjectUtils; import com.kingsrook.qqq.backend.core.utils.Pair; import com.kingsrook.qqq.backend.core.utils.StringUtils; import com.kingsrook.qqq.backend.core.utils.ValueUtils; @@ -150,6 +154,7 @@ public class ApiImplementation QTableMetaData table = validateTableAndVersion(apiInstanceMetaData, version, tableApiName, ApiOperation.QUERY_BY_QUERY_STRING); String tableName = table.getName(); + String apiName = apiInstanceMetaData.getName(); QueryInput queryInput = new QueryInput(); queryInput.setTableName(tableName); @@ -231,6 +236,8 @@ public class ApiImplementation badRequestMessages.add("includeCount must be either true or false"); } + Map tableApiFields = GetTableApiFieldsAction.getTableApiFieldMap(new GetTableApiFieldsAction.ApiNameVersionAndTableName(apiName, version, tableName)); + if(StringUtils.hasContent(orderBy)) { for(String orderByPart : orderBy.split(",")) @@ -238,6 +245,7 @@ public class ApiImplementation orderByPart = orderByPart.trim(); String[] orderByNameDirection = orderByPart.split(" +"); boolean asc = true; + String apiFieldName = orderByNameDirection[0]; if(orderByNameDirection.length == 2) { if("asc".equalsIgnoreCase(orderByNameDirection[1])) @@ -250,7 +258,7 @@ public class ApiImplementation } else { - badRequestMessages.add("orderBy direction for field " + orderByNameDirection[0] + " must be either ASC or DESC."); + badRequestMessages.add("orderBy direction for field " + apiFieldName + " must be either ASC or DESC."); } } else if(orderByNameDirection.length > 2) @@ -258,14 +266,27 @@ public class ApiImplementation badRequestMessages.add("Unrecognized format for orderBy clause: " + orderByPart + ". Expected: fieldName [ASC|DESC]."); } - try + QFieldMetaData field = tableApiFields.get(apiFieldName); + if(field == null) { - QFieldMetaData field = table.getField(orderByNameDirection[0]); - filter.withOrderBy(new QFilterOrderBy(field.getName(), asc)); + badRequestMessages.add("Unrecognized orderBy field name: " + apiFieldName + "."); } - catch(Exception e) + else { - badRequestMessages.add("Unrecognized orderBy field name: " + orderByNameDirection[0] + "."); + QFilterOrderBy filterOrderBy = new QFilterOrderBy(field.getName(), asc); + + ApiFieldMetaData apiFieldMetaData = ObjectUtils.tryAndRequireNonNullElse(() -> ApiFieldMetaDataContainer.of(field).getApiFieldMetaData(apiName), new ApiFieldMetaData()); + if(StringUtils.hasContent(apiFieldMetaData.getReplacedByFieldName())) + { + filterOrderBy.setFieldName(apiFieldMetaData.getReplacedByFieldName()); + } + else if(apiFieldMetaData.getCustomValueMapper() != null) + { + ApiFieldCustomValueMapper customValueMapper = QCodeLoader.getAdHoc(ApiFieldCustomValueMapper.class, apiFieldMetaData.getCustomValueMapper()); + customValueMapper.customizeFilterOrderBy(queryInput, filterOrderBy, apiFieldName, apiFieldMetaData); + } + + filter.withOrderBy(filterOrderBy); } } } @@ -289,20 +310,36 @@ public class ApiImplementation continue; } - try + QFieldMetaData field = tableApiFields.get(name); + if(field == null) { - //////////////////////////////////////////////////////////////////////////////////////////////// - // todo - deal with removed fields; fields w/ custom value mappers (need new method(s) there) // - //////////////////////////////////////////////////////////////////////////////////////////////// - - QFieldMetaData field = table.getField(name); + badRequestMessages.add("Unrecognized filter criteria field: " + name); + } + else + { + ApiFieldMetaData apiFieldMetaData = ObjectUtils.tryAndRequireNonNullElse(() -> ApiFieldMetaDataContainer.of(field).getApiFieldMetaData(apiName), new ApiFieldMetaData()); for(String value : values) { if(StringUtils.hasContent(value)) { try { - filter.addCriteria(parseQueryParamToCriteria(field, name, value)); + QFilterCriteria criteria = parseQueryParamToCriteria(field, name, value); + + ///////////////////////////////////////////// + // deal with replaced or customized fields // + ///////////////////////////////////////////// + if(StringUtils.hasContent(apiFieldMetaData.getReplacedByFieldName())) + { + criteria.setFieldName(apiFieldMetaData.getReplacedByFieldName()); + } + else if(apiFieldMetaData.getCustomValueMapper() != null) + { + ApiFieldCustomValueMapper customValueMapper = QCodeLoader.getAdHoc(ApiFieldCustomValueMapper.class, apiFieldMetaData.getCustomValueMapper()); + customValueMapper.customizeFilterCriteria(queryInput, filter, criteria, name, apiFieldMetaData); + } + + filter.addCriteria(criteria); } catch(Exception e) { @@ -311,10 +348,6 @@ public class ApiImplementation } } } - catch(Exception e) - { - badRequestMessages.add("Unrecognized filter criteria field: " + name); - } } ////////////////////////////////////////// @@ -350,7 +383,7 @@ public class ApiImplementation ArrayList> records = new ArrayList<>(); for(QRecord record : queryOutput.getRecords()) { - records.add(QRecordApiAdapter.qRecordToApiMap(record, tableName, apiInstanceMetaData.getName(), version)); + records.add(QRecordApiAdapter.qRecordToApiMap(record, tableName, apiName, version)); } ///////////////////////////// diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GenerateOpenApiSpecAction.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GenerateOpenApiSpecAction.java index 8783965e..e6d53c19 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GenerateOpenApiSpecAction.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GenerateOpenApiSpecAction.java @@ -485,7 +485,7 @@ public class GenerateOpenApiSpecAction extends AbstractQActionFunction parameters = new ArrayList<>(); ApiProcessInput apiProcessInput = apiProcessMetaData.getInput(); + String apiName = apiInstanceMetaData.getName(); if(apiProcessInput != null) { ApiProcessInputFieldsContainer queryStringParams = apiProcessInput.getQueryStringParams(); @@ -912,12 +915,13 @@ public class GenerateOpenApiSpecAction extends AbstractQActionFunction buildOrderByExamples(String primaryKeyApiName, List tableApiFields) + private Map buildOrderByExamples(String apiName, String primaryKeyApiName, List tableApiFields) { Map rs = new LinkedHashMap<>(); @@ -1569,7 +1577,7 @@ public class GenerateOpenApiSpecAction extends AbstractQActionFunction fieldsForExample5 = new ArrayList<>(); for(QFieldMetaData tableApiField : tableApiFields) { - String name = tableApiField.getName(); + String name = ApiFieldMetaData.getEffectiveApiFieldName(apiName, tableApiField); if(primaryKeyApiName.equals(name) || fieldsForExample4.contains(name) || fieldsForExample5.contains(name)) { continue; diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GetTableApiFieldsAction.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GetTableApiFieldsAction.java index f4807b1e..23cfdf48 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GetTableApiFieldsAction.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/GetTableApiFieldsAction.java @@ -24,7 +24,10 @@ package com.kingsrook.qqq.api.actions; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import com.kingsrook.qqq.api.model.APIVersion; import com.kingsrook.qqq.api.model.APIVersionRange; import com.kingsrook.qqq.api.model.actions.GetTableApiFieldsInput; @@ -50,6 +53,65 @@ import org.apache.commons.lang.BooleanUtils; *******************************************************************************/ public class GetTableApiFieldsAction extends AbstractQActionFunction { + private static Map> fieldListCache = new HashMap<>(); + private static Map> fieldMapCache = new HashMap<>(); + + + + /******************************************************************************* + ** Allow tests (that manipulate meta-data) to clear field caches. + *******************************************************************************/ + public static void clearCaches() + { + fieldListCache.clear(); + fieldMapCache.clear(); + } + + + + /******************************************************************************* + ** convenience (and caching) wrapper + *******************************************************************************/ + public static Map getTableApiFieldMap(ApiNameVersionAndTableName apiNameVersionAndTableName) throws QException + { + if(!fieldMapCache.containsKey(apiNameVersionAndTableName)) + { + Map map = getTableApiFieldList(apiNameVersionAndTableName).stream().collect(Collectors.toMap(f -> (ApiFieldMetaData.getEffectiveApiFieldName(apiNameVersionAndTableName.apiName(), f)), f -> f)); + fieldMapCache.put(apiNameVersionAndTableName, map); + } + + return (fieldMapCache.get(apiNameVersionAndTableName)); + } + + + + /******************************************************************************* + ** convenience (and caching) wrapper + *******************************************************************************/ + public static List getTableApiFieldList(ApiNameVersionAndTableName apiNameVersionAndTableName) throws QException + { + if(!fieldListCache.containsKey(apiNameVersionAndTableName)) + { + List value = new GetTableApiFieldsAction().execute(new GetTableApiFieldsInput() + .withTableName(apiNameVersionAndTableName.tableName()) + .withVersion(apiNameVersionAndTableName.apiVersion()) + .withApiName(apiNameVersionAndTableName.apiName())).getFields(); + fieldListCache.put(apiNameVersionAndTableName, value); + } + return (fieldListCache.get(apiNameVersionAndTableName)); + } + + + + /******************************************************************************* + ** Input-record for convenience methods + *******************************************************************************/ + public record ApiNameVersionAndTableName(String apiName, String apiVersion, String tableName) + { + + } + + /******************************************************************************* ** diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java index 8b16305a..da2e211a 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/actions/QRecordApiAdapter.java @@ -29,12 +29,10 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import com.kingsrook.qqq.api.javalin.QBadRequestException; import com.kingsrook.qqq.api.model.APIVersion; import com.kingsrook.qqq.api.model.APIVersionRange; import com.kingsrook.qqq.api.model.actions.ApiFieldCustomValueMapper; -import com.kingsrook.qqq.api.model.actions.GetTableApiFieldsInput; import com.kingsrook.qqq.api.model.metadata.ApiInstanceMetaData; import com.kingsrook.qqq.api.model.metadata.ApiInstanceMetaDataContainer; import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaData; @@ -66,20 +64,6 @@ public class QRecordApiAdapter { private static final QLogger LOG = QLogger.getLogger(QRecordApiAdapter.class); - private static Map> fieldListCache = new HashMap<>(); - private static Map> fieldMapCache = new HashMap<>(); - - - - /******************************************************************************* - ** Allow tests (that manipulate meta-data) to clear field caches. - *******************************************************************************/ - public static void clearCaches() - { - fieldListCache.clear(); - fieldMapCache.clear(); - } - /******************************************************************************* @@ -92,7 +76,7 @@ public class QRecordApiAdapter return (null); } - List tableApiFields = getTableApiFieldList(new ApiNameVersionAndTableName(apiName, apiVersion, tableName)); + List tableApiFields = GetTableApiFieldsAction.getTableApiFieldList(new GetTableApiFieldsAction.ApiNameVersionAndTableName(apiName, apiVersion, tableName)); LinkedHashMap outputRecord = new LinkedHashMap<>(); ///////////////////////////////////////// @@ -111,7 +95,7 @@ public class QRecordApiAdapter else if(apiFieldMetaData.getCustomValueMapper() != null) { ApiFieldCustomValueMapper customValueMapper = QCodeLoader.getAdHoc(ApiFieldCustomValueMapper.class, apiFieldMetaData.getCustomValueMapper()); - value = customValueMapper.produceApiValue(record); + value = customValueMapper.produceApiValue(record, apiFieldName); } else { @@ -185,7 +169,7 @@ public class QRecordApiAdapter //////////////////////////////////////////////////////////////////////////////// // make map of apiFieldNames (e.g., names as api uses them) to QFieldMetaData // //////////////////////////////////////////////////////////////////////////////// - Map apiFieldsMap = getTableApiFieldMap(new ApiNameVersionAndTableName(apiName, apiVersion, tableName)); + Map apiFieldsMap = GetTableApiFieldsAction.getTableApiFieldMap(new GetTableApiFieldsAction.ApiNameVersionAndTableName(apiName, apiVersion, tableName)); List unrecognizedFieldNames = new ArrayList<>(); QRecord qRecord = new QRecord(); @@ -241,7 +225,7 @@ public class QRecordApiAdapter else if(apiFieldMetaData.getCustomValueMapper() != null) { ApiFieldCustomValueMapper customValueMapper = QCodeLoader.getAdHoc(ApiFieldCustomValueMapper.class, apiFieldMetaData.getCustomValueMapper()); - customValueMapper.consumeApiValue(qRecord, value, jsonObject); + customValueMapper.consumeApiValue(qRecord, value, jsonObject, jsonKey); } else { @@ -332,7 +316,7 @@ public class QRecordApiAdapter { if(!supportedVersion.toString().equals(apiVersion)) { - Map versionFields = getTableApiFieldMap(new ApiNameVersionAndTableName(apiName, supportedVersion.toString(), tableName)); + Map versionFields = GetTableApiFieldsAction.getTableApiFieldMap(new GetTableApiFieldsAction.ApiNameVersionAndTableName(apiName, supportedVersion.toString(), tableName)); if(versionFields.containsKey(unrecognizedFieldName)) { versionsWithThisField.add(supportedVersion.toString()); @@ -348,47 +332,4 @@ public class QRecordApiAdapter return (null); } - - - /******************************************************************************* - ** - *******************************************************************************/ - private static Map getTableApiFieldMap(ApiNameVersionAndTableName apiNameVersionAndTableName) throws QException - { - if(!fieldMapCache.containsKey(apiNameVersionAndTableName)) - { - Map map = getTableApiFieldList(apiNameVersionAndTableName).stream().collect(Collectors.toMap(f -> (ApiFieldMetaData.getEffectiveApiFieldName(apiNameVersionAndTableName.apiName(), f)), f -> f)); - fieldMapCache.put(apiNameVersionAndTableName, map); - } - - return (fieldMapCache.get(apiNameVersionAndTableName)); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - private static List getTableApiFieldList(ApiNameVersionAndTableName apiNameVersionAndTableName) throws QException - { - if(!fieldListCache.containsKey(apiNameVersionAndTableName)) - { - List value = new GetTableApiFieldsAction().execute(new GetTableApiFieldsInput() - .withTableName(apiNameVersionAndTableName.tableName()) - .withVersion(apiNameVersionAndTableName.apiVersion()) - .withApiName(apiNameVersionAndTableName.apiName())).getFields(); - fieldListCache.put(apiNameVersionAndTableName, value); - } - return (fieldListCache.get(apiNameVersionAndTableName)); - } - - - - /******************************************************************************* - ** - *******************************************************************************/ - private record ApiNameVersionAndTableName(String apiName, String apiVersion, String tableName) - { - - } } diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/actions/ApiFieldCustomValueMapper.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/actions/ApiFieldCustomValueMapper.java index 26f0b3cd..36f27359 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/actions/ApiFieldCustomValueMapper.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/actions/ApiFieldCustomValueMapper.java @@ -23,6 +23,11 @@ package com.kingsrook.qqq.api.model.actions; import java.io.Serializable; +import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaData; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; import com.kingsrook.qqq.backend.core.model.data.QRecord; import org.json.JSONObject; @@ -34,9 +39,11 @@ public abstract class ApiFieldCustomValueMapper { /******************************************************************************* - ** + ** When producing a JSON Object to send over the API (e.g., for a GET), this method + ** can run to customize the value that is produced, for the input QRecord's specified + ** fieldName *******************************************************************************/ - public Serializable produceApiValue(QRecord record) + public Serializable produceApiValue(QRecord record, String apiFieldName) { ///////////////////// // null by default // @@ -46,10 +53,36 @@ public abstract class ApiFieldCustomValueMapper + /******************************************************************************* + ** When producing a QRecord (the first parameter) from a JSON Object that was + ** received from the API (e.g., a POST or PATCH) - this method can run to + ** allow customization of the incoming value. + *******************************************************************************/ + public void consumeApiValue(QRecord record, Object value, JSONObject fullApiJsonObject, String apiFieldName) + { + ///////////////////// + // noop by default // + ///////////////////// + } + + + /******************************************************************************* ** *******************************************************************************/ - public void consumeApiValue(QRecord record, Object value, JSONObject fullApiJsonObject) + public void customizeFilterCriteria(QueryInput queryInput, QQueryFilter filter, QFilterCriteria criteria, String apiFieldName, ApiFieldMetaData apiFieldMetaData) + { + ///////////////////// + // noop by default // + ///////////////////// + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + public void customizeFilterOrderBy(QueryInput queryInput, QFilterOrderBy orderBy, String apiFieldName, ApiFieldMetaData apiFieldMetaData) { ///////////////////// // noop by default // diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaData.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaData.java index 0a496b1e..de0ecc2e 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaData.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaData.java @@ -35,6 +35,7 @@ import com.kingsrook.qqq.api.model.metadata.ApiOperation; import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaData; import com.kingsrook.qqq.api.model.metadata.fields.ApiFieldMetaDataContainer; import com.kingsrook.qqq.backend.core.instances.QInstanceEnricher; +import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -80,7 +81,7 @@ public class ApiTableMetaData implements ApiOperation.EnabledOperationsProvider /******************************************************************************* ** *******************************************************************************/ - public void enrich(String apiName, QTableMetaData table) + public void enrich(QInstance qInstance, String apiName, QTableMetaData table) { if(initialVersion != null) { @@ -95,7 +96,7 @@ public class ApiTableMetaData implements ApiOperation.EnabledOperationsProvider for(QFieldMetaData field : CollectionUtils.nonNullList(removedApiFields)) { - new QInstanceEnricher(null).enrichField(field); + new QInstanceEnricher(qInstance).enrichField(field); ApiFieldMetaData apiFieldMetaData = ensureFieldHasApiSupplementalMetaData(apiName, field); if(apiFieldMetaData.getInitialVersion() == null) { diff --git a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaDataContainer.java b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaDataContainer.java index 784dba02..daf44695 100644 --- a/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaDataContainer.java +++ b/qqq-middleware-api/src/main/java/com/kingsrook/qqq/api/model/metadata/tables/ApiTableMetaDataContainer.java @@ -25,6 +25,7 @@ package com.kingsrook.qqq.api.model.metadata.tables; import java.util.LinkedHashMap; import java.util.Map; import com.kingsrook.qqq.api.ApiSupplementType; +import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.tables.QSupplementalTableMetaData; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -80,13 +81,13 @@ public class ApiTableMetaDataContainer extends QSupplementalTableMetaData ** *******************************************************************************/ @Override - public void enrich(QTableMetaData table) + public void enrich(QInstance qInstance, QTableMetaData table) { - super.enrich(table); + super.enrich(qInstance, table); for(Map.Entry entry : CollectionUtils.nonNullMap(apis).entrySet()) { - entry.getValue().enrich(entry.getKey(), table); + entry.getValue().enrich(qInstance, entry.getKey(), table); } } diff --git a/qqq-middleware-api/src/test/java/com/kingsrook/qqq/api/actions/ApiImplementationTest.java b/qqq-middleware-api/src/test/java/com/kingsrook/qqq/api/actions/ApiImplementationTest.java index 7b9d854c..a55b2027 100644 --- a/qqq-middleware-api/src/test/java/com/kingsrook/qqq/api/actions/ApiImplementationTest.java +++ b/qqq-middleware-api/src/test/java/com/kingsrook/qqq/api/actions/ApiImplementationTest.java @@ -23,9 +23,14 @@ package com.kingsrook.qqq.api.actions; import java.io.Serializable; +import java.math.BigDecimal; +import java.time.LocalDate; +import java.time.Month; +import java.util.List; import java.util.Map; import com.kingsrook.qqq.api.BaseTest; import com.kingsrook.qqq.api.TestUtils; +import com.kingsrook.qqq.api.javalin.QBadRequestException; import com.kingsrook.qqq.api.model.actions.ApiFieldCustomValueMapper; import com.kingsrook.qqq.api.model.metadata.ApiInstanceMetaData; import com.kingsrook.qqq.api.model.metadata.ApiInstanceMetaDataContainer; @@ -35,19 +40,23 @@ import com.kingsrook.qqq.api.model.metadata.tables.ApiAssociationMetaData; import com.kingsrook.qqq.api.model.metadata.tables.ApiTableMetaData; import com.kingsrook.qqq.api.model.metadata.tables.ApiTableMetaDataContainer; import com.kingsrook.qqq.backend.core.actions.tables.GetAction; +import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.model.actions.tables.get.GetInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.ValueUtils; +import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; import org.json.JSONObject; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -66,7 +75,7 @@ class ApiImplementationTest extends BaseTest @AfterEach void beforeAndAfterEach() { - QRecordApiAdapter.clearCaches(); + GetTableApiFieldsAction.clearCaches(); } @@ -188,6 +197,43 @@ class ApiImplementationTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testQueryWithRemovedFields() throws QException + { + QInstance qInstance = QContext.getQInstance(); + ApiInstanceMetaData apiInstanceMetaData = ApiInstanceMetaDataContainer.of(qInstance).getApiInstanceMetaData(TestUtils.API_NAME); + + new InsertAction().execute(new InsertInput(TestUtils.TABLE_NAME_PERSON).withRecord(new QRecord() + .withValue("firstName", "Tim") + .withValue("noOfShoes", 2) + .withValue("birthDate", LocalDate.of(1980, Month.MAY, 31)) + .withValue("cost", new BigDecimal("3.50")) + .withValue("price", new BigDecimal("9.99")) + .withValue("photo", "ABCD".getBytes()))); + + /////////////////////////////////////////////////////////////////////////////////////////////// + // query by a field that wasn't in an old api version, but is in the table now - should fail // + /////////////////////////////////////////////////////////////////////////////////////////////// + + assertThatThrownBy(() -> + ApiImplementation.query(apiInstanceMetaData, TestUtils.V2022_Q4, TestUtils.TABLE_NAME_PERSON, MapBuilder.of("noOfShoes", List.of("2")))) + .isInstanceOf(QBadRequestException.class) + .hasMessageContaining("Unrecognized filter criteria field"); + + { + ///////////////////////////////////////////// + // query by a removed field (was replaced) // + ///////////////////////////////////////////// + Map queryResult = ApiImplementation.query(apiInstanceMetaData, TestUtils.V2022_Q4, TestUtils.TABLE_NAME_PERSON, MapBuilder.of("shoeCount", List.of("2"))); + assertEquals(1, queryResult.get("count")); + } + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -198,7 +244,7 @@ class ApiImplementationTest extends BaseTest ** *******************************************************************************/ @Override - public Serializable produceApiValue(QRecord record) + public Serializable produceApiValue(QRecord record, String apiFieldName) { return ("customValue-" + record.getValueString("lastName")); } @@ -209,7 +255,7 @@ class ApiImplementationTest extends BaseTest ** *******************************************************************************/ @Override - public void consumeApiValue(QRecord record, Object value, JSONObject fullApiJsonObject) + public void consumeApiValue(QRecord record, Object value, JSONObject fullApiJsonObject, String apiFieldName) { String valueString = ValueUtils.getValueAsString(value); valueString = valueString.replaceFirst("^stripThisAway-", ""); From c5489522810cee62bc51d537203982ffc59e56ba Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 13:18:13 -0500 Subject: [PATCH 04/13] Fixing a case in query joins, where a joinMetaData was given, but it needed flipped. --- .../actions/tables/query/JoinsContext.java | 211 +++++++++++++----- .../qqq/backend/module/rdbms/TestUtils.java | 27 +++ .../rdbms/actions/RDBMSQueryActionTest.java | 49 ++++ .../test/resources/prime-test-database.sql | 25 ++- 4 files changed, 255 insertions(+), 57 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 3d39455a..1284ea0a 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 @@ -30,8 +30,10 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.logging.LogPair; import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; @@ -41,6 +43,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.tables.ExposedJoin; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.collections.MutableList; +import org.apache.logging.log4j.Level; import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair; @@ -60,6 +63,7 @@ public class JoinsContext // note - will have entries for all tables, not just aliases. // //////////////////////////////////////////////////////////////// private final Map aliasToTableNameMap = new HashMap<>(); + private Level logLevel = Level.OFF; @@ -69,12 +73,14 @@ public class JoinsContext *******************************************************************************/ public JoinsContext(QInstance instance, String tableName, List queryJoins, QQueryFilter filter) throws QException { + log("--- START ----------------------------------------------------------------------", logPair("mainTable", tableName)); this.instance = instance; this.mainTableName = tableName; this.queryJoins = new MutableList<>(queryJoins); for(QueryJoin queryJoin : this.queryJoins) { + log("Processing input query join", logPair("joinTable", queryJoin.getJoinTable()), logPair("alias", queryJoin.getAlias()), logPair("baseTableOrAlias", queryJoin.getBaseTableOrAlias()), logPair("joinMetaDataName", () -> queryJoin.getJoinMetaData().getName())); processQueryJoin(queryJoin); } @@ -83,48 +89,7 @@ public class JoinsContext /////////////////////////////////////////////////////////////// for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(instance.getTable(tableName).getRecordSecurityLocks())) { - /////////////////////////////////////////////////////////////////////////////////////////////////// - // ok - so - the join name chain is going to be like this: // - // for a table: orderLineItemExtrinsic (that's 2 away from order, where the security field is): // - // - securityFieldName = order.clientId // - // - joinNameChain = orderJoinOrderLineItem, orderLineItemJoinOrderLineItemExtrinsic // - // so - to navigate from the table to the security field, we need to reverse the joinNameChain, // - // and step (via tmpTable variable) back to the securityField // - /////////////////////////////////////////////////////////////////////////////////////////////////// - ArrayList joinNameChain = new ArrayList<>(CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain())); - Collections.reverse(joinNameChain); - - QTableMetaData tmpTable = instance.getTable(mainTableName); - - for(String joinName : joinNameChain) - { - if(this.queryJoins.stream().anyMatch(queryJoin -> - { - QJoinMetaData joinMetaData = Objects.requireNonNullElseGet(queryJoin.getJoinMetaData(), () -> findJoinMetaData(instance, tableName, queryJoin.getJoinTable())); - return (joinMetaData != null && Objects.equals(joinMetaData.getName(), joinName)); - })) - { - continue; - } - - QJoinMetaData join = instance.getJoin(joinName); - if(join.getLeftTable().equals(tmpTable.getName())) - { - QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join).withType(QueryJoin.Type.INNER); - this.addQueryJoin(queryJoin); - tmpTable = instance.getTable(join.getRightTable()); - } - else if(join.getRightTable().equals(tmpTable.getName())) - { - QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join.flip()).withType(QueryJoin.Type.INNER); - this.addQueryJoin(queryJoin); // - tmpTable = instance.getTable(join.getLeftTable()); - } - else - { - throw (new QException("Error adding security lock joins to query - table name [" + tmpTable.getName() + "] not found in join [" + joinName + "]")); - } - } + ensureRecordSecurityLockIsRepresented(instance, tableName, recordSecurityLock); } ensureFilterIsRepresented(filter); @@ -141,6 +106,86 @@ public class JoinsContext } } */ + + log("Constructed JoinsContext", logPair("mainTableName", this.mainTableName), logPair("queryJoins", this.queryJoins.stream().map(qj -> qj.getJoinTable()).collect(Collectors.joining(",")))); + log("--- END ------------------------------------------------------------------------"); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private void ensureRecordSecurityLockIsRepresented(QInstance instance, String tableName, RecordSecurityLock recordSecurityLock) throws QException + { + /////////////////////////////////////////////////////////////////////////////////////////////////// + // ok - so - the join name chain is going to be like this: // + // for a table: orderLineItemExtrinsic (that's 2 away from order, where the security field is): // + // - securityFieldName = order.clientId // + // - joinNameChain = orderJoinOrderLineItem, orderLineItemJoinOrderLineItemExtrinsic // + // so - to navigate from the table to the security field, we need to reverse the joinNameChain, // + // and step (via tmpTable variable) back to the securityField // + /////////////////////////////////////////////////////////////////////////////////////////////////// + ArrayList joinNameChain = new ArrayList<>(CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain())); + Collections.reverse(joinNameChain); + log("Evaluating recordSecurityLock", logPair("recordSecurityLock", recordSecurityLock.getFieldName()), logPair("joinNameChain", joinNameChain)); + + QTableMetaData tmpTable = instance.getTable(mainTableName); + + for(String joinName : joinNameChain) + { + /////////////////////////////////////////////////////////////////////////////////////////////////////// + // check the joins currently in the query - if any are for this table, then we don't need to add one // + /////////////////////////////////////////////////////////////////////////////////////////////////////// + List matchingJoins = this.queryJoins.stream().filter(queryJoin -> + { + QJoinMetaData joinMetaData = null; + if(queryJoin.getJoinMetaData() != null) + { + joinMetaData = queryJoin.getJoinMetaData(); + } + else + { + joinMetaData = findJoinMetaData(instance, tableName, queryJoin.getJoinTable()); + } + return (joinMetaData != null && Objects.equals(joinMetaData.getName(), joinName)); + }).toList(); + + if(CollectionUtils.nullSafeHasContents(matchingJoins)) + { + ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // note - if a user added a join as an outer type, we need to change it to be inner, for the security purpose. // + // todo - is this always right? what about nulls-allowed? // + ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// + log("- skipping join already in the query", logPair("joinName", joinName)); + + if(matchingJoins.get(0).getType().equals(QueryJoin.Type.LEFT) || matchingJoins.get(0).getType().equals(QueryJoin.Type.RIGHT)) + { + log("- - although... it was here as an outer - so switching it to INNER", logPair("joinName", joinName)); + matchingJoins.get(0).setType(QueryJoin.Type.INNER); + } + + continue; + } + + QJoinMetaData join = instance.getJoin(joinName); + if(join.getLeftTable().equals(tmpTable.getName())) + { + QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join).withType(QueryJoin.Type.INNER); + this.addQueryJoin(queryJoin, "forRecordSecurityLock (non-flipped)"); + tmpTable = instance.getTable(join.getRightTable()); + } + else if(join.getRightTable().equals(tmpTable.getName())) + { + QueryJoin queryJoin = new ImplicitQueryJoinForSecurityLock().withJoinMetaData(join.flip()).withType(QueryJoin.Type.INNER); + this.addQueryJoin(queryJoin, "forRecordSecurityLock (flipped)"); + tmpTable = instance.getTable(join.getLeftTable()); + } + else + { + throw (new QException("Error adding security lock joins to query - table name [" + tmpTable.getName() + "] not found in join [" + joinName + "]")); + } + } } @@ -151,8 +196,15 @@ public class JoinsContext ** use this method to add to the list, instead of ever adding directly, as it's ** important do to that process step (and we've had bugs when it wasn't done). *******************************************************************************/ - private void addQueryJoin(QueryJoin queryJoin) throws QException + private void addQueryJoin(QueryJoin queryJoin, String reason) throws QException { + log("Adding query join to context", + logPair("reason", reason), + logPair("joinTable", queryJoin.getJoinTable()), + logPair("joinMetaData.name", () -> queryJoin.getJoinMetaData().getName()), + logPair("joinMetaData.leftTable", () -> queryJoin.getJoinMetaData().getLeftTable()), + logPair("joinMetaData.rightTable", () -> queryJoin.getJoinMetaData().getRightTable()) + ); this.queryJoins.add(queryJoin); processQueryJoin(queryJoin); } @@ -177,10 +229,46 @@ public class JoinsContext addedJoin = false; for(QueryJoin queryJoin : queryJoins) { - ///////////////////////////////////////////////////////////////////// - // if the join has joinMetaData, then we don't need to process it. // - ///////////////////////////////////////////////////////////////////// - if(queryJoin.getJoinMetaData() == null) + /////////////////////////////////////////////////////////////////////////////////////////////// + // if the join has joinMetaData, then we don't need to process it... unless it needs flipped // + /////////////////////////////////////////////////////////////////////////////////////////////// + QJoinMetaData joinMetaData = queryJoin.getJoinMetaData(); + if(joinMetaData != null) + { + boolean isJoinLeftTableInQuery = false; + String joinMetaDataLeftTable = joinMetaData.getLeftTable(); + if(joinMetaDataLeftTable.equals(mainTableName)) + { + isJoinLeftTableInQuery = true; + } + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // check the other joins in this query - if any of them have this join's left-table as their baseTable, then set the flag to true // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + for(QueryJoin otherJoin : queryJoins) + { + if(otherJoin == queryJoin) + { + continue; + } + + if(Objects.equals(otherJoin.getBaseTableOrAlias(), joinMetaDataLeftTable)) + { + isJoinLeftTableInQuery = true; + break; + } + } + + ///////////////////////////////////////////////////////////////////////////////// + // if the join's left-table isn't in the query, then we need to flip the join. // + ///////////////////////////////////////////////////////////////////////////////// + if(!isJoinLeftTableInQuery) + { + log("Flipping queryJoin because its leftTable wasn't found in the query", logPair("joinMetaDataName", joinMetaData.getName()), logPair("leftTable", joinMetaDataLeftTable)); + queryJoin.setJoinMetaData(joinMetaData.flip()); + } + } + else { ////////////////////////////////////////////////////////////////////// // try to find a direct join between the main table and this table. // @@ -190,6 +278,7 @@ public class JoinsContext QJoinMetaData found = findJoinMetaData(instance, baseTableName, queryJoin.getJoinTable()); if(found != null) { + log("Found joinMetaData - setting it in queryJoin", logPair("joinMetaDataName", found.getName()), logPair("baseTableName", baseTableName), logPair("joinTable", queryJoin.getJoinTable())); queryJoin.setJoinMetaData(found); } else @@ -197,15 +286,13 @@ public class JoinsContext ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// // else, the join must be indirect - so look for an exposedJoin that will have a joinPath that will connect us // ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// - LOG.debug("Looking for an exposed join...", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable())); - QTableMetaData mainTable = instance.getTable(mainTableName); boolean addedAnyQueryJoins = false; for(ExposedJoin exposedJoin : CollectionUtils.nonNullList(mainTable.getExposedJoins())) { if(queryJoin.getJoinTable().equals(exposedJoin.getJoinTable())) { - LOG.debug("Found an exposed join", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable()), logPair("joinPath", exposedJoin.getJoinPath())); + log("Found an exposed join", logPair("mainTable", mainTableName), logPair("joinTable", queryJoin.getJoinTable()), logPair("joinPath", exposedJoin.getJoinPath())); ///////////////////////////////////////////////////////////////////////////////////// // loop backward through the join path (from the joinTable back to the main table) // @@ -250,7 +337,7 @@ public class JoinsContext QueryJoin queryJoinToAdd = makeQueryJoinFromJoinAndTableNames(nextTable, tmpTable, joinToAdd); queryJoinToAdd.setType(queryJoin.getType()); addedAnyQueryJoins = true; - this.addQueryJoin(queryJoinToAdd); + this.addQueryJoin(queryJoinToAdd, "forExposedJoin"); } } @@ -377,9 +464,9 @@ public class JoinsContext ** ** e.g., Given: ** FROM `order` INNER JOIN line_item li - ** hasAliasOrTable("order") => true - ** hasAliasOrTable("li") => false - ** hasAliasOrTable("line_item") => true + ** hasTable("order") => true + ** hasTable("li") => false + ** hasTable("line_item") => true *******************************************************************************/ public boolean hasTable(String table) { @@ -415,15 +502,17 @@ public class JoinsContext for(String filterTable : filterTables) { + log("Evaluating filterTable", logPair("filterTable", filterTable)); if(!aliasToTableNameMap.containsKey(filterTable) && !Objects.equals(mainTableName, filterTable)) { + log("- table not in query - adding it", logPair("filterTable", filterTable)); boolean found = false; for(QJoinMetaData join : CollectionUtils.nonNullMap(QContext.getQInstance().getJoins()).values()) { QueryJoin queryJoin = makeQueryJoinFromJoinAndTableNames(mainTableName, filterTable, join); if(queryJoin != null) { - this.addQueryJoin(queryJoin); + this.addQueryJoin(queryJoin, "forFilter (join found in instance)"); found = true; break; } @@ -432,7 +521,7 @@ public class JoinsContext if(!found) { QueryJoin queryJoin = new QueryJoin().withJoinTable(filterTable).withType(QueryJoin.Type.INNER); - this.addQueryJoin(queryJoin); + this.addQueryJoin(queryJoin, "forFilter (join not found in instance)"); } } } @@ -569,4 +658,14 @@ public class JoinsContext { } + + + /******************************************************************************* + ** + *******************************************************************************/ + private void log(String message, LogPair... logPairs) + { + LOG.log(logLevel, message, null, logPairs); + } + } diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/TestUtils.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/TestUtils.java index 2a9b32f4..d882807a 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/TestUtils.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/TestUtils.java @@ -66,6 +66,7 @@ public class TestUtils public static final String TABLE_NAME_PERSONAL_ID_CARD = "personalIdCard"; public static final String TABLE_NAME_STORE = "store"; public static final String TABLE_NAME_ORDER = "order"; + public static final String TABLE_NAME_ORDER_INSTRUCTIONS = "orderInstructions"; public static final String TABLE_NAME_ITEM = "item"; public static final String TABLE_NAME_ORDER_LINE = "orderLine"; public static final String TABLE_NAME_LINE_ITEM_EXTRINSIC = "orderLineExtrinsic"; @@ -245,6 +246,16 @@ public class TestUtils .withField(new QFieldMetaData("storeId", QFieldType.INTEGER).withBackendName("store_id").withPossibleValueSourceName(TABLE_NAME_STORE)) .withField(new QFieldMetaData("billToPersonId", QFieldType.INTEGER).withBackendName("bill_to_person_id").withPossibleValueSourceName(TABLE_NAME_PERSON)) .withField(new QFieldMetaData("shipToPersonId", QFieldType.INTEGER).withBackendName("ship_to_person_id").withPossibleValueSourceName(TABLE_NAME_PERSON)) + .withField(new QFieldMetaData("currentOrderInstructionsId", QFieldType.INTEGER).withBackendName("current_order_instructions_id").withPossibleValueSourceName(TABLE_NAME_PERSON)) + ); + + qInstance.addTable(defineBaseTable(TABLE_NAME_ORDER_INSTRUCTIONS, "order_instructions") + .withRecordSecurityLock(new RecordSecurityLock() + .withSecurityKeyType(TABLE_NAME_STORE) + .withFieldName("order.storeId") + .withJoinNameChain(List.of("orderInstructionsJoinOrder"))) + .withField(new QFieldMetaData("orderId", QFieldType.INTEGER).withBackendName("order_id")) + .withField(new QFieldMetaData("instructions", QFieldType.STRING)) ); qInstance.addTable(defineBaseTable(TABLE_NAME_ITEM, "item") @@ -357,6 +368,22 @@ public class TestUtils .withJoinOn(new JoinOn("id", "orderLineId")) ); + qInstance.addJoin(new QJoinMetaData() + .withName("orderJoinCurrentOrderInstructions") + .withLeftTable(TABLE_NAME_ORDER) + .withRightTable(TABLE_NAME_ORDER_INSTRUCTIONS) + .withType(JoinType.ONE_TO_ONE) + .withJoinOn(new JoinOn("currentOrderInstructionsId", "id")) + ); + + qInstance.addJoin(new QJoinMetaData() + .withName("orderInstructionsJoinOrder") + .withLeftTable(TABLE_NAME_ORDER_INSTRUCTIONS) + .withRightTable(TABLE_NAME_ORDER) + .withType(JoinType.MANY_TO_ONE) + .withJoinOn(new JoinOn("orderId", "id")) + ); + qInstance.addPossibleValueSource(new QPossibleValueSource() .withName("store") .withType(QPossibleValueSourceType.TABLE) diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java index b8558e12..01c2c65c 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/actions/RDBMSQueryActionTest.java @@ -32,10 +32,12 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; +import com.kingsrook.qqq.backend.core.actions.tables.CountAction; import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria; @@ -1695,4 +1697,51 @@ public class RDBMSQueryActionTest extends RDBMSActionTest } + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testMultipleReversedDirectionJoinsBetweenSameTables() throws QException + { + QContext.setQSession(new QSession().withSecurityKeyValue(TestUtils.SECURITY_KEY_STORE_ALL_ACCESS, true)); + + { + ///////////////////////////////////////////////////////// + // assert a failure if the join to use isn't specified // + ///////////////////////////////////////////////////////// + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(TestUtils.TABLE_NAME_ORDER); + queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS)); + assertThatThrownBy(() -> new QueryAction().execute(queryInput)).rootCause().hasMessageContaining("More than 1 join was found"); + } + + Integer noOfOrders = new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_ORDER)).getCount(); + Integer noOfOrderInstructions = new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS)).getCount(); + + { + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // make sure we can join on order.current_order_instruction_id = order_instruction.id -- and that we get back 1 row per order // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(TestUtils.TABLE_NAME_ORDER); + queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS).withJoinMetaData(QContext.getQInstance().getJoin("orderJoinCurrentOrderInstructions"))); + QueryOutput queryOutput = new QueryAction().execute(queryInput); + assertEquals(noOfOrders, queryOutput.getRecords().size()); + } + + { + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // make sure we can join on order.id = order_instruction.order_id -- and that we get back 1 row per order instruction // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + QueryInput queryInput = new QueryInput(); + queryInput.setTableName(TestUtils.TABLE_NAME_ORDER); + queryInput.withQueryJoin(new QueryJoin(TestUtils.TABLE_NAME_ORDER_INSTRUCTIONS).withJoinMetaData(QContext.getQInstance().getJoin("orderInstructionsJoinOrder"))); + QueryOutput queryOutput = new QueryAction().execute(queryInput); + assertEquals(noOfOrderInstructions, queryOutput.getRecords().size()); + } + + } + } diff --git a/qqq-backend-module-rdbms/src/test/resources/prime-test-database.sql b/qqq-backend-module-rdbms/src/test/resources/prime-test-database.sql index 87620a4c..974ac099 100644 --- a/qqq-backend-module-rdbms/src/test/resources/prime-test-database.sql +++ b/qqq-backend-module-rdbms/src/test/resources/prime-test-database.sql @@ -84,6 +84,7 @@ DROP TABLE IF EXISTS line_item_extrinsic; DROP TABLE IF EXISTS order_line; DROP TABLE IF EXISTS item; DROP TABLE IF EXISTS `order`; +DROP TABLE IF EXISTS order_instructions; DROP TABLE IF EXISTS warehouse_store_int; DROP TABLE IF EXISTS store; DROP TABLE IF EXISTS warehouse; @@ -123,7 +124,8 @@ CREATE TABLE `order` id INT AUTO_INCREMENT PRIMARY KEY, store_id INT REFERENCES store, bill_to_person_id INT, - ship_to_person_id INT + ship_to_person_id INT, + current_order_instructions_id INT -- f-key to order_instructions, which also has an f-key back here! ); -- variable orders @@ -136,6 +138,27 @@ INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES (7, 3, null, 5); INSERT INTO `order` (id, store_id, bill_to_person_id, ship_to_person_id) VALUES (8, 3, null, 5); +CREATE TABLE order_instructions +( + id INT AUTO_INCREMENT PRIMARY KEY, + order_id INT, + instructions VARCHAR(250) +); + +-- give orders 1 & 2 multiple versions of the instruction record +INSERT INTO order_instructions (id, order_id, instructions) VALUES (1, 1, 'order 1 v1'); +INSERT INTO order_instructions (id, order_id, instructions) VALUES (2, 1, 'order 1 v2'); +UPDATE `order` SET current_order_instructions_id = 2 WHERE id=1; + +INSERT INTO order_instructions (id, order_id, instructions) VALUES (3, 2, 'order 2 v1'); +INSERT INTO order_instructions (id, order_id, instructions) VALUES (4, 2, 'order 2 v2'); +INSERT INTO order_instructions (id, order_id, instructions) VALUES (5, 2, 'order 2 v3'); +UPDATE `order` SET current_order_instructions_id = 5 WHERE id=2; + +-- give all other orders just 1 instruction +INSERT INTO order_instructions (order_id, instructions) SELECT id, concat('order ', id, ' v1') FROM `order` WHERE current_order_instructions_id IS NULL; +UPDATE `order` SET current_order_instructions_id = (SELECT MIN(id) FROM order_instructions WHERE order_id = `order`.id) WHERE current_order_instructions_id is null; + CREATE TABLE order_line ( id INT AUTO_INCREMENT PRIMARY KEY, From 3406929e75dae382ca57114e0c863572b05e0b26 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 13:18:27 -0500 Subject: [PATCH 05/13] process query joins in Get --- .../kingsrook/qqq/backend/javalin/QJavalinImplementation.java | 2 ++ 1 file changed, 2 insertions(+) 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 172d9544..29ec751b 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 @@ -871,6 +871,8 @@ public class QJavalinImplementation getInput.setShouldTranslatePossibleValues(true); getInput.setShouldFetchHeavyFields(true); + getInput.setQueryJoins(processQueryJoinsParam(context)); + if("true".equals(context.queryParam("includeAssociations"))) { getInput.setIncludeAssociations(true); From 05f23410993deba1dae8302ded05690b9a9ac039 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 16:21:28 -0500 Subject: [PATCH 06/13] CE-607 Instance validation for section-fields from join tables --- .../core/instances/QInstanceValidator.java | 36 +++++++++++--- .../instances/QInstanceValidatorTest.java | 49 +++++++++++++++++++ .../qqq/backend/core/utils/TestUtils.java | 1 + 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java index e18cb182..7c253031 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java @@ -438,10 +438,13 @@ public class QInstanceValidator for(QFieldSection section : table.getSections()) { validateTableSection(qInstance, table, section, fieldNamesInSections); - if(section.getTier().equals(Tier.T1)) + if(assertCondition(section.getTier() != null, "Table " + tableName + " " + section.getName() + " is missing its tier")) { - assertCondition(tier1Section == null, "Table " + tableName + " has more than 1 section listed as Tier 1"); - tier1Section = section; + if(section.getTier().equals(Tier.T1)) + { + assertCondition(tier1Section == null, "Table " + tableName + " has more than 1 section listed as Tier 1"); + tier1Section = section; + } } assertCondition(!usedSectionNames.contains(section.getName()), "Table " + tableName + " has more than 1 section named " + section.getName()); @@ -1099,13 +1102,34 @@ public class QInstanceValidator boolean hasFields = CollectionUtils.nullSafeHasContents(section.getFieldNames()); boolean hasWidget = StringUtils.hasContent(section.getWidgetName()); - if(assertCondition(hasFields || hasWidget, "Table " + table.getName() + " section " + section.getName() + " does not have any fields or a widget.")) + String sectionPrefix = "Table " + table.getName() + " section " + section.getName() + " "; + if(assertCondition(hasFields || hasWidget, sectionPrefix + "does not have any fields or a widget.")) { if(table.getFields() != null && hasFields) { for(String fieldName : section.getFieldNames()) { - assertCondition(table.getFields().containsKey(fieldName), "Table " + table.getName() + " section " + section.getName() + " specifies fieldName " + fieldName + ", which is not a field on this table."); + if(fieldName.contains(".")) + { + String[] parts = fieldName.split("\\."); + String otherTableName = parts[0]; + String foreignFieldName = parts[1]; + + if(assertCondition(qInstance.getTable(otherTableName) != null, sectionPrefix + "join-field " + fieldName + ", which is referencing an unrecognized table name [" + otherTableName + "]")) + { + List matchedExposedJoins = CollectionUtils.nonNullList(table.getExposedJoins()).stream().filter(ej -> otherTableName.equals(ej.getJoinTable())).toList(); + if(assertCondition(CollectionUtils.nullSafeHasContents(matchedExposedJoins), sectionPrefix + "join-field " + fieldName + ", referencing table [" + otherTableName + "] which is not an exposed join on this table.")) + { + assertCondition(!matchedExposedJoins.get(0).getIsMany(), sectionPrefix + "join-field " + fieldName + " references an is-many join, which is not supported."); + } + assertCondition(qInstance.getTable(otherTableName).getFields().containsKey(foreignFieldName), sectionPrefix + "join-field " + fieldName + " specifies a fieldName [" + foreignFieldName + "] which does not exist in that table [" + otherTableName + "]."); + } + } + else + { + assertCondition(table.getFields().containsKey(fieldName), sectionPrefix + "specifies fieldName " + fieldName + ", which is not a field on this table."); + } + assertCondition(!fieldNamesInSections.contains(fieldName), "Table " + table.getName() + " has field " + fieldName + " listed more than once in its field sections."); fieldNamesInSections.add(fieldName); @@ -1113,7 +1137,7 @@ public class QInstanceValidator } else if(hasWidget) { - assertCondition(qInstance.getWidget(section.getWidgetName()) != null, "Table " + table.getName() + " section " + section.getName() + " specifies widget " + section.getWidgetName() + ", which is not a widget in this instance."); + assertCondition(qInstance.getWidget(section.getWidgetName()) != null, sectionPrefix + "specifies widget " + section.getWidgetName() + ", which is not a widget in this instance."); } } } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java index 62d14fbb..84e48161 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java @@ -822,6 +822,55 @@ class QInstanceValidatorTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testSectionsWithJoinFields() + { + Consumer putAllFieldsInASection = table -> table.addSection(new QFieldSection() + .withName("section0") + .withTier(Tier.T1) + .withFieldNames(new ArrayList<>(table.getFields().keySet()))); + + assertValidationFailureReasons(qInstance -> + { + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_ORDER); + putAllFieldsInASection.accept(table); + table.getSections().get(0).getFieldNames().add(TestUtils.TABLE_NAME_LINE_ITEM + ".sku"); + }, "orderLine.sku references an is-many join, which is not supported"); + + assertValidationSuccess(qInstance -> + { + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); + putAllFieldsInASection.accept(table); + table.getSections().get(0).getFieldNames().add(TestUtils.TABLE_NAME_ORDER + ".orderNo"); + }); + + assertValidationFailureReasons(qInstance -> + { + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); + putAllFieldsInASection.accept(table); + table.getSections().get(0).getFieldNames().add(TestUtils.TABLE_NAME_ORDER + ".asdf"); + }, "order.asdf specifies a fieldName [asdf] which does not exist in that table [order]."); + + assertValidationFailureReasons(qInstance -> + { + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); + putAllFieldsInASection.accept(table); + table.getSections().get(0).getFieldNames().add("foo.bar"); + }, "unrecognized table name [foo]"); + + assertValidationFailureReasons(qInstance -> + { + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); + putAllFieldsInASection.accept(table); + table.getSections().get(0).getFieldNames().add(TestUtils.TABLE_NAME_SHAPE + ".id"); + }, "[shape] which is not an exposed join on this table"); + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java index a38be542..21942e00 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java @@ -590,6 +590,7 @@ public class TestUtils .withFieldName("order.storeId") .withJoinNameChain(List.of("orderLineItem"))) .withAssociation(new Association().withName("extrinsics").withAssociatedTableName(TABLE_NAME_LINE_ITEM_EXTRINSIC).withJoinName("lineItemLineItemExtrinsic")) + .withExposedJoin(new ExposedJoin().withJoinTable(TABLE_NAME_ORDER)) .withField(new QFieldMetaData("id", QFieldType.INTEGER).withIsEditable(false)) .withField(new QFieldMetaData("createDate", QFieldType.DATE_TIME).withIsEditable(false)) .withField(new QFieldMetaData("modifyDate", QFieldType.DATE_TIME).withIsEditable(false)) From 5dfa10912edbc8e318052296ce1d4d3f5848a738 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 16:45:30 -0500 Subject: [PATCH 07/13] CE-607 Slight tweaks to exposed join field validation --- .../core/instances/QInstanceValidator.java | 9 +++++++-- .../core/model/metadata/tables/ExposedJoin.java | 15 +++++++++++---- .../core/instances/QInstanceValidatorTest.java | 15 +++++++++------ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java index 7c253031..38aae827 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java @@ -1109,7 +1109,12 @@ public class QInstanceValidator { for(String fieldName : section.getFieldNames()) { - if(fieldName.contains(".")) + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // note - this was originally written as an assertion: // + // if(assertCondition(qInstance.getTable(otherTableName) != null, sectionPrefix + "join-field " + fieldName + ", which is referencing an unrecognized table name [" + otherTableName + "]")) // + // but... then a field name with dots gives us a bad time here, so... // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + if(fieldName.contains(".") && qInstance.getTable(fieldName.split("\\.")[0]) != null) { String[] parts = fieldName.split("\\."); String otherTableName = parts[0]; @@ -1120,7 +1125,7 @@ public class QInstanceValidator List matchedExposedJoins = CollectionUtils.nonNullList(table.getExposedJoins()).stream().filter(ej -> otherTableName.equals(ej.getJoinTable())).toList(); if(assertCondition(CollectionUtils.nullSafeHasContents(matchedExposedJoins), sectionPrefix + "join-field " + fieldName + ", referencing table [" + otherTableName + "] which is not an exposed join on this table.")) { - assertCondition(!matchedExposedJoins.get(0).getIsMany(), sectionPrefix + "join-field " + fieldName + " references an is-many join, which is not supported."); + assertCondition(!matchedExposedJoins.get(0).getIsMany(qInstance), sectionPrefix + "join-field " + fieldName + " references an is-many join, which is not supported."); } assertCondition(qInstance.getTable(otherTableName).getFields().containsKey(foreignFieldName), sectionPrefix + "join-field " + fieldName + " specifies a fieldName [" + foreignFieldName + "] which does not exist in that table [" + otherTableName + "]."); } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java index f66b1b23..6c3fa7b0 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java @@ -62,7 +62,17 @@ public class ExposedJoin /******************************************************************************* ** *******************************************************************************/ - public Boolean getIsMany() + public boolean getIsMany() + { + return (getIsMany(QContext.getQInstance())); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + public Boolean getIsMany(QInstance qInstance) { if(isMany == null) { @@ -70,8 +80,6 @@ public class ExposedJoin { try { - QInstance qInstance = QContext.getQInstance(); - ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // loop backward through the joinPath, starting at the join table (since we don't know the table that this exposedJoin is attached to!) // ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -204,5 +212,4 @@ public class ExposedJoin this.joinPath = joinPath; return (this); } - } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java index 84e48161..f66dfa16 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidatorTest.java @@ -854,12 +854,15 @@ class QInstanceValidatorTest extends BaseTest table.getSections().get(0).getFieldNames().add(TestUtils.TABLE_NAME_ORDER + ".asdf"); }, "order.asdf specifies a fieldName [asdf] which does not exist in that table [order]."); - assertValidationFailureReasons(qInstance -> - { - QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); - putAllFieldsInASection.accept(table); - table.getSections().get(0).getFieldNames().add("foo.bar"); - }, "unrecognized table name [foo]"); + ///////////////////////////////////////////////////////////////////////////// + // this is aactually allowed, well, just not considered as a join-field... // + ///////////////////////////////////////////////////////////////////////////// + // assertValidationFailureReasons(qInstance -> + // { + // QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_LINE_ITEM); + // putAllFieldsInASection.accept(table); + // table.getSections().get(0).getFieldNames().add("foo.bar"); + // }, "unrecognized table name [foo]"); assertValidationFailureReasons(qInstance -> { From 1da85ce0a23e95ad0c21ef1515815968c83904fe Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 8 Aug 2023 16:51:47 -0500 Subject: [PATCH 08/13] CE-607 Go go tests --- .../qqq/backend/core/model/metadata/tables/ExposedJoin.java | 3 +++ .../com/kingsrook/qqq/backend/core/utils/YamlUtilsTest.java | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java index 6c3fa7b0..67dd503e 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/tables/ExposedJoin.java @@ -23,6 +23,7 @@ package com.kingsrook.qqq.backend.core.model.metadata.tables; import java.util.List; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; @@ -62,6 +63,7 @@ public class ExposedJoin /******************************************************************************* ** *******************************************************************************/ + @JsonIgnore public boolean getIsMany() { return (getIsMany(QContext.getQInstance())); @@ -72,6 +74,7 @@ public class ExposedJoin /******************************************************************************* ** *******************************************************************************/ + @JsonIgnore public Boolean getIsMany(QInstance qInstance) { if(isMany == null) diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/YamlUtilsTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/YamlUtilsTest.java index a41afa18..3c1c7c95 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/YamlUtilsTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/YamlUtilsTest.java @@ -24,6 +24,7 @@ package com.kingsrook.qqq.backend.core.utils; import java.util.Map; import com.fasterxml.jackson.core.JsonProcessingException; +import com.kingsrook.qqq.backend.core.BaseTest; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import org.junit.jupiter.api.Test; @@ -31,7 +32,7 @@ import org.junit.jupiter.api.Test; /******************************************************************************* ** Unit test for YamlUtils *******************************************************************************/ -class YamlUtilsTest +class YamlUtilsTest extends BaseTest { /******************************************************************************* From d4e18d8f5506c4f4df5a8da54155687280824108 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Mon, 14 Aug 2023 19:37:00 -0500 Subject: [PATCH 09/13] CE-608: updated check for jsonObject when processing API GET request to consider the object being jsonObject.isNull(), added ability to use CUSTOM authorization in an API util override --- .../module/api/actions/BaseAPIActionUtil.java | 18 +++++++++++++++++- .../module/api/model/AuthorizationType.java | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) 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 cc5d35fb..7dd9f6ca 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 @@ -519,7 +519,7 @@ public class BaseAPIActionUtil { String wrapperObjectName = getBackendDetails(table).getTableWrapperObjectName(); jsonObject = JsonUtils.toJSONObject(resultString); - if(jsonObject.has(wrapperObjectName)) + if(jsonObject.has(wrapperObjectName) && !jsonObject.isNull(wrapperObjectName)) { Object o = jsonObject.get(wrapperObjectName); if(o instanceof JSONArray jsonArray) @@ -750,6 +750,10 @@ public class BaseAPIActionUtil throw (new QException("Error setting authorization query parameter", e)); } } + case CUSTOM -> + { + handleCustomAuthorization(request); + } default -> throw new IllegalArgumentException("Unexpected authorization type: " + backendMetaData.getAuthorizationType()); } } @@ -1398,4 +1402,16 @@ public class BaseAPIActionUtil /////////////////////////////////////////////////////////////////////////////////////////////////// return (7 * 1024); } + + + + /******************************************************************************* + ** + *******************************************************************************/ + protected void handleCustomAuthorization(HttpRequestBase request) throws QException + { + /////////////////////////////////////////////////////////////////////// + // nothing to do at this layer, meant to be overridden by subclasses // + /////////////////////////////////////////////////////////////////////// + } } diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/model/AuthorizationType.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/model/AuthorizationType.java index 7e8060f3..9a4f750c 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/model/AuthorizationType.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/model/AuthorizationType.java @@ -31,6 +31,7 @@ public enum AuthorizationType API_TOKEN, BASIC_AUTH_API_KEY, BASIC_AUTH_USERNAME_PASSWORD, + CUSTOM, OAUTH2, API_KEY_QUERY_PARAM, } From 0d0ab6c2e5bdab4a1c1e6514efe82de30e896a62 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 15 Aug 2023 16:55:36 -0500 Subject: [PATCH 10/13] CE-567 Add concept of security lock Scope - e.g., READ-WRITE (blocking all access to a record), or just WRITE - which means anyone can read, but you must have the key to WRITE. --- .../core/actions/audits/AuditAction.java | 3 +- .../core/actions/audits/DMLAuditAction.java | 3 +- .../core/actions/tables/DeleteAction.java | 3 + .../core/actions/tables/UpdateAction.java | 25 +++++- .../ValidateRecordSecurityLockHelper.java | 26 ++++-- .../core/instances/QInstanceValidator.java | 2 + .../actions/audits/AuditSingleInput.java | 3 +- .../actions/tables/query/JoinsContext.java | 3 +- .../metadata/security/RecordSecurityLock.java | 48 ++++++++++ .../security/RecordSecurityLockFilters.java | 80 +++++++++++++++++ .../StoreScriptRevisionProcessStep.java | 23 ++++- .../core/actions/tables/DeleteActionTest.java | 31 +++++++ .../core/actions/tables/InsertActionTest.java | 59 +++++++++++++ .../core/actions/tables/UpdateActionTest.java | 87 ++++++++++++++++++- .../qqq/backend/core/utils/TestUtils.java | 41 +++++++++ .../rdbms/actions/AbstractRDBMSAction.java | 7 +- 16 files changed, 420 insertions(+), 24 deletions(-) create mode 100644 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLockFilters.java 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 41015e57..b8b3d4e0 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 @@ -44,6 +44,7 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertOutput; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.model.session.QUser; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -166,7 +167,7 @@ public class AuditAction extends AbstractQActionFunction getRecordSecurityKeyValues(QTableMetaData table, QRecord record) { Map securityKeyValues = new HashMap<>(); - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(table.getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(table.getRecordSecurityLocks()))) { securityKeyValues.put(recordSecurityLock.getSecurityKeyType(), record == null ? null : record.getValue(recordSecurityLock.getFieldName())); } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteAction.java index ca9badc9..ee49499a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteAction.java @@ -40,6 +40,7 @@ import com.kingsrook.qqq.backend.core.actions.customizers.AbstractPreDeleteCusto import com.kingsrook.qqq.backend.core.actions.customizers.QCodeLoader; import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizers; import com.kingsrook.qqq.backend.core.actions.interfaces.DeleteInterface; +import com.kingsrook.qqq.backend.core.actions.tables.helpers.ValidateRecordSecurityLockHelper; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.logging.LogPair; @@ -321,6 +322,8 @@ public class DeleteAction QTableMetaData table = deleteInput.getTable(); List primaryKeysNotFound = validateRecordsExistAndCanBeAccessed(deleteInput, oldRecordList.get()); + ValidateRecordSecurityLockHelper.validateSecurityFields(table, oldRecordList.get(), ValidateRecordSecurityLockHelper.Action.DELETE); + /////////////////////////////////////////////////////////////////////////// // after all validations, run the pre-delete customizer, if there is one // /////////////////////////////////////////////////////////////////////////// diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateAction.java index c45197f7..a36decd0 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateAction.java @@ -61,6 +61,8 @@ 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.joins.JoinOn; import com.kingsrook.qqq.backend.core.model.metadata.joins.QJoinMetaData; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.Association; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.model.statusmessages.BadInputStatusMessage; @@ -209,14 +211,16 @@ public class UpdateAction { validateRecordsExistAndCanBeAccessed(updateInput, oldRecordList.get()); } + else + { + ValidateRecordSecurityLockHelper.validateSecurityFields(table, updateInput.getRecords(), ValidateRecordSecurityLockHelper.Action.UPDATE); + } if(updateInput.getInputSource().shouldValidateRequiredFields()) { validateRequiredFields(updateInput); } - ValidateRecordSecurityLockHelper.validateSecurityFields(table, updateInput.getRecords(), ValidateRecordSecurityLockHelper.Action.UPDATE); - /////////////////////////////////////////////////////////////////////////// // after all validations, run the pre-update customizer, if there is one // /////////////////////////////////////////////////////////////////////////// @@ -287,6 +291,8 @@ public class UpdateAction QTableMetaData table = updateInput.getTable(); QFieldMetaData primaryKeyField = table.getField(table.getPrimaryKeyField()); + List onlyWriteLocks = RecordSecurityLockFilters.filterForOnlyWriteLocks(CollectionUtils.nonNullList(table.getRecordSecurityLocks())); + for(List page : CollectionUtils.getPages(updateInput.getRecords(), 1000)) { List primaryKeysToLookup = new ArrayList<>(); @@ -320,6 +326,8 @@ public class UpdateAction } } + ValidateRecordSecurityLockHelper.validateSecurityFields(table, updateInput.getRecords(), ValidateRecordSecurityLockHelper.Action.UPDATE); + for(QRecord record : page) { Serializable value = ValueUtils.getValueAsFieldType(primaryKeyField.getType(), record.getValue(table.getPrimaryKeyField())); @@ -332,6 +340,19 @@ public class UpdateAction { record.addError(new NotFoundStatusMessage("No record was found to update for " + primaryKeyField.getLabel() + " = " + value)); } + else + { + /////////////////////////////////////////////////////////////////////////////////////////// + // if the table has any write-only locks, validate their values here, on the old-records // + /////////////////////////////////////////////////////////////////////////////////////////// + for(RecordSecurityLock lock : onlyWriteLocks) + { + QRecord oldRecord = lookedUpRecords.get(value); + QFieldType fieldType = table.getField(lock.getFieldName()).getType(); + Serializable lockValue = ValueUtils.getValueAsFieldType(fieldType, oldRecord.getValue(lock.getFieldName())); + ValidateRecordSecurityLockHelper.validateRecordSecurityValue(table, record, lock, lockValue, fieldType, ValidateRecordSecurityLockHelper.Action.UPDATE); + } + } } } } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/helpers/ValidateRecordSecurityLockHelper.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/helpers/ValidateRecordSecurityLockHelper.java index 041ed54a..e94c9b5a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/helpers/ValidateRecordSecurityLockHelper.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/tables/helpers/ValidateRecordSecurityLockHelper.java @@ -44,6 +44,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.joins.JoinOn; import com.kingsrook.qqq.backend.core.model.metadata.joins.QJoinMetaData; import com.kingsrook.qqq.backend.core.model.metadata.security.QSecurityKeyType; import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.model.statusmessages.PermissionDeniedMessage; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -68,6 +69,7 @@ public class ValidateRecordSecurityLockHelper { INSERT, UPDATE, + DELETE, SELECT } @@ -78,7 +80,7 @@ public class ValidateRecordSecurityLockHelper *******************************************************************************/ public static void validateSecurityFields(QTableMetaData table, List records, Action action) throws QException { - List locksToCheck = getRecordSecurityLocks(table); + List locksToCheck = getRecordSecurityLocks(table, action); if(CollectionUtils.nullSafeIsEmpty(locksToCheck)) { return; @@ -98,11 +100,12 @@ public class ValidateRecordSecurityLockHelper for(QRecord record : records) { - if(action.equals(Action.UPDATE) && !record.getValues().containsKey(field.getName())) + if(action.equals(Action.UPDATE) && !record.getValues().containsKey(field.getName()) && RecordSecurityLock.LockScope.READ_AND_WRITE.equals(recordSecurityLock.getLockScope())) { - ///////////////////////////////////////////////////////////////////////// - // if not updating the security field, then no error can come from it! // - ///////////////////////////////////////////////////////////////////////// + ///////////////////////////////////////////////////////////////////////////////////////////////////////// + // if this is a read-write lock, then if we have the record, it means we were able to read the record. // + // So if we're not updating the security field, then no error can come from it! // + ///////////////////////////////////////////////////////////////////////////////////////////////////////// continue; } @@ -244,11 +247,18 @@ public class ValidateRecordSecurityLockHelper /******************************************************************************* ** *******************************************************************************/ - private static List getRecordSecurityLocks(QTableMetaData table) + private static List getRecordSecurityLocks(QTableMetaData table, Action action) { - List recordSecurityLocks = table.getRecordSecurityLocks(); + List recordSecurityLocks = CollectionUtils.nonNullList(table.getRecordSecurityLocks()); List locksToCheck = new ArrayList<>(); + recordSecurityLocks = switch(action) + { + case INSERT, UPDATE, DELETE -> RecordSecurityLockFilters.filterForWriteLocks(recordSecurityLocks); + case SELECT -> RecordSecurityLockFilters.filterForReadLocks(recordSecurityLocks); + default -> throw (new IllegalArgumentException("Unsupported action: " + action)); + }; + //////////////////////////////////////// // if there are no locks, just return // //////////////////////////////////////// @@ -281,7 +291,7 @@ public class ValidateRecordSecurityLockHelper /******************************************************************************* ** *******************************************************************************/ - static void validateRecordSecurityValue(QTableMetaData table, QRecord record, RecordSecurityLock recordSecurityLock, Serializable recordSecurityValue, QFieldType fieldType, Action action) + public static void validateRecordSecurityValue(QTableMetaData table, QRecord record, RecordSecurityLock recordSecurityLock, Serializable recordSecurityValue, QFieldType fieldType, Action action) { if(recordSecurityValue == null) { diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java index e18cb182..baa5a67e 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java @@ -586,6 +586,8 @@ public class QInstanceValidator prefix = "Table " + table.getName() + " recordSecurityLock (of key type " + securityKeyTypeName + ") "; + assertCondition(recordSecurityLock.getLockScope() != null, prefix + " is missing its lockScope"); + boolean hasAnyBadJoins = false; for(String joinName : CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain())) { diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/audits/AuditSingleInput.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/audits/AuditSingleInput.java index 2711f3d4..655d31d0 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/audits/AuditSingleInput.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/audits/AuditSingleInput.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -246,7 +247,7 @@ public class AuditSingleInput setAuditTableName(table.getName()); this.securityKeyValues = new HashMap<>(); - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(table.getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(table.getRecordSecurityLocks()))) { this.securityKeyValues.put(recordSecurityLock.getFieldName(), record.getValueInteger(recordSecurityLock.getFieldName())); } 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 3d39455a..b48547f0 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 @@ -37,6 +37,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.joins.QJoinMetaData; import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.ExposedJoin; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; @@ -81,7 +82,7 @@ public class JoinsContext /////////////////////////////////////////////////////////////// // ensure any joins that contribute a recordLock are present // /////////////////////////////////////////////////////////////// - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(instance.getTable(tableName).getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(instance.getTable(tableName).getRecordSecurityLocks()))) { /////////////////////////////////////////////////////////////////////////////////////////////////// // ok - so - the join name chain is going to be like this: // diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLock.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLock.java index beac4587..06633998 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLock.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLock.java @@ -34,6 +34,10 @@ import java.util.List; ** - recordSecurityLock.fieldName = order.clientId ** - recordSecurityLock.joinNameChain = [orderJoinOrderLineItem, orderLineItemJoinOrderLineItemExtrinsic] ** that is - what's the chain that takes us FROM the security fieldName TO the table with the lock. + ** + ** LockScope controls what the lock prevents users from doing without a valid key. + ** - READ_AND_WRITE means that users cannot read or write records without a valid key. + ** - WRITE means that users cannot write records without a valid key (but they can read them). *******************************************************************************/ public class RecordSecurityLock { @@ -42,6 +46,8 @@ public class RecordSecurityLock private List joinNameChain; private NullValueBehavior nullValueBehavior = NullValueBehavior.DENY; + private LockScope lockScope = LockScope.READ_AND_WRITE; + /******************************************************************************* @@ -66,6 +72,17 @@ public class RecordSecurityLock + /******************************************************************************* + ** + *******************************************************************************/ + public enum LockScope + { + READ_AND_WRITE, + WRITE + } + + + /******************************************************************************* ** Getter for securityKeyType *******************************************************************************/ @@ -188,4 +205,35 @@ public class RecordSecurityLock return (this); } + + + /******************************************************************************* + ** Getter for lockScope + *******************************************************************************/ + public LockScope getLockScope() + { + return (this.lockScope); + } + + + + /******************************************************************************* + ** Setter for lockScope + *******************************************************************************/ + public void setLockScope(LockScope lockScope) + { + this.lockScope = lockScope; + } + + + + /******************************************************************************* + ** Fluent setter for lockScope + *******************************************************************************/ + public RecordSecurityLock withLockScope(LockScope lockScope) + { + this.lockScope = lockScope; + return (this); + } + } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLockFilters.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLockFilters.java new file mode 100644 index 00000000..c8c7e9dc --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/security/RecordSecurityLockFilters.java @@ -0,0 +1,80 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2023. 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.metadata.security; + + +import java.util.List; + + +/******************************************************************************* + ** standard filtering operations for lists of record security locks. + *******************************************************************************/ +public class RecordSecurityLockFilters +{ + + /******************************************************************************* + ** filter a list of locks so that we only see the ones that apply to reads. + *******************************************************************************/ + public static List filterForReadLocks(List recordSecurityLocks) + { + if(recordSecurityLocks == null) + { + return (null); + } + + return (recordSecurityLocks.stream().filter(rsl -> RecordSecurityLock.LockScope.READ_AND_WRITE.equals(rsl.getLockScope())).toList()); + } + + + + /******************************************************************************* + ** filter a list of locks so that we only see the ones that apply to writes. + *******************************************************************************/ + public static List filterForWriteLocks(List recordSecurityLocks) + { + if(recordSecurityLocks == null) + { + return (null); + } + + return (recordSecurityLocks.stream().filter(rsl -> + RecordSecurityLock.LockScope.READ_AND_WRITE.equals(rsl.getLockScope()) + || RecordSecurityLock.LockScope.WRITE.equals(rsl.getLockScope() + )).toList()); + } + + + + /******************************************************************************* + ** filter a list of locks so that we only see the ones that are WRITE type only. + *******************************************************************************/ + public static List filterForOnlyWriteLocks(List recordSecurityLocks) + { + if(recordSecurityLocks == null) + { + return (null); + } + + return (recordSecurityLocks.stream().filter(rsl -> RecordSecurityLock.LockScope.WRITE.equals(rsl.getLockScope())).toList()); + } + +} diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/scripts/StoreScriptRevisionProcessStep.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/scripts/StoreScriptRevisionProcessStep.java index f7b94ab9..4893920a 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/scripts/StoreScriptRevisionProcessStep.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/processes/implementations/scripts/StoreScriptRevisionProcessStep.java @@ -31,7 +31,10 @@ import com.kingsrook.qqq.backend.core.actions.tables.GetAction; import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.actions.tables.UpdateAction; +import com.kingsrook.qqq.backend.core.actions.tables.helpers.ValidateRecordSecurityLockHelper; +import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.exceptions.QPermissionDeniedException; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepInput; import com.kingsrook.qqq.backend.core.model.actions.processes.RunBackendStepOutput; import com.kingsrook.qqq.backend.core.model.actions.tables.get.GetInput; @@ -46,6 +49,8 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryOutput; import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput; import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.model.scripts.Script; +import com.kingsrook.qqq.backend.core.model.scripts.ScriptRevision; import com.kingsrook.qqq.backend.core.model.scripts.ScriptRevisionFile; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; @@ -69,7 +74,7 @@ public class StoreScriptRevisionProcessStep implements BackendStep { InsertAction insertAction = new InsertAction(); InsertInput insertInput = new InsertInput(); - insertInput.setTableName("scriptRevision"); + insertInput.setTableName(ScriptRevision.TABLE_NAME); QBackendTransaction transaction = insertAction.openTransaction(insertInput); insertInput.setTransaction(transaction); @@ -87,14 +92,23 @@ public class StoreScriptRevisionProcessStep implements BackendStep // get the existing script, to update // //////////////////////////////////////// GetInput getInput = new GetInput(); - getInput.setTableName("script"); + getInput.setTableName(Script.TABLE_NAME); getInput.setPrimaryKey(scriptId); getInput.setTransaction(transaction); GetOutput getOutput = new GetAction().execute(getInput); QRecord script = getOutput.getRecord(); + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // in case the app added a security field to the scripts table, make sure the user is allowed to edit the script // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + ValidateRecordSecurityLockHelper.validateSecurityFields(QContext.getQInstance().getTable(Script.TABLE_NAME), List.of(script), ValidateRecordSecurityLockHelper.Action.UPDATE); + if(CollectionUtils.nullSafeHasContents(script.getErrors())) + { + throw (new QPermissionDeniedException(script.getErrors().get(0).getMessage())); + } + QueryInput queryInput = new QueryInput(); - queryInput.setTableName("scriptRevision"); + queryInput.setTableName(ScriptRevision.TABLE_NAME); queryInput.setFilter(new QQueryFilter() .withCriteria(new QFilterCriteria("scriptId", QCriteriaOperator.EQUALS, List.of(script.getValue("id")))) .withOrderBy(new QFilterOrderBy("sequenceNo", false)) @@ -183,7 +197,7 @@ public class StoreScriptRevisionProcessStep implements BackendStep //////////////////////////////////////////////////// script.setValue("currentScriptRevisionId", scriptRevision.getValue("id")); UpdateInput updateInput = new UpdateInput(); - updateInput.setTableName("script"); + updateInput.setTableName(Script.TABLE_NAME); updateInput.setRecords(List.of(script)); updateInput.setTransaction(transaction); new UpdateAction().execute(updateInput); @@ -198,6 +212,7 @@ public class StoreScriptRevisionProcessStep implements BackendStep catch(Exception e) { transaction.rollback(); + throw (e); } finally { diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteActionTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteActionTest.java index 076a3f39..b2abd0a8 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteActionTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/DeleteActionTest.java @@ -53,6 +53,7 @@ import com.kingsrook.qqq.backend.core.utils.TestUtils; import com.kingsrook.qqq.backend.core.utils.collections.ListBuilder; import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -365,6 +366,36 @@ class DeleteActionTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testSecurityLockWriteScope() throws QException + { + TestUtils.updatePersonMemoryTableInContextWithWritableByWriteLockAndInsert3TestRecords(); + + ///////////////////////////////////////////////////////////////////////////////////////////////////////////// + // try to delete 1, 2, and 3. 2 should be blocked, because it has a writable-By that isn't in our session // + ///////////////////////////////////////////////////////////////////////////////////////////////////////////// + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("jdoe"))); + DeleteInput deleteInput = new DeleteInput(); + deleteInput.setTableName(TestUtils.TABLE_NAME_PERSON_MEMORY); + deleteInput.setPrimaryKeys(List.of(1, 2, 3)); + DeleteOutput deleteOutput = new DeleteAction().execute(deleteInput); + + assertEquals(1, deleteOutput.getRecordsWithErrors().size()); + assertThat(deleteOutput.getRecordsWithErrors().get(0).getErrors().get(0).getMessage()) + .contains("You do not have permission") + .contains("kmarsh") + .contains("Only Writable By"); + + assertEquals(1, new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_PERSON_MEMORY)).getCount()); + assertEquals(1, new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_PERSON_MEMORY) + .withFilter(new QQueryFilter(new QFilterCriteria("id", QCriteriaOperator.EQUALS, 2)))).getCount()); + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/InsertActionTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/InsertActionTest.java index f83b10c0..2fa44957 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/InsertActionTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/InsertActionTest.java @@ -35,9 +35,12 @@ 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.security.RecordSecurityLock; import com.kingsrook.qqq.backend.core.model.metadata.tables.UniqueKey; +import com.kingsrook.qqq.backend.core.model.statusmessages.QErrorMessage; import com.kingsrook.qqq.backend.core.modules.backend.implementations.memory.MemoryRecordStore; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.TestUtils; +import com.kingsrook.qqq.backend.core.utils.collections.ListBuilder; +import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -692,4 +695,60 @@ class InsertActionTest extends BaseTest assertEquals(3, TestUtils.queryTable(qInstance, TestUtils.TABLE_NAME_ORDER).size()); } + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testSecurityLockWriteScope() throws QException + { + TestUtils.updatePersonMemoryTableInContextWithWritableByWriteLockAndInsert3TestRecords(); + + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("hsimpson"))); + + ///////////////////////////////////////////////////////////////////////////////////////// + // with only hsimpson in our key, make sure we can't insert a row w/ a different value // + ///////////////////////////////////////////////////////////////////////////////////////// + { + InsertOutput insertOutput = new InsertAction().execute(new InsertInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 100).withValue("firstName", "Jean-Luc").withValue("onlyWritableBy", "jkirk") + ))); + List errors = insertOutput.getRecords().get(0).getErrors(); + assertEquals(1, errors.size()); + assertThat(errors.get(0).getMessage()) + .contains("You do not have permission") + .contains("jkirk") + .contains("Only Writable By"); + } + + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // make sure we can insert w/ a null in onlyWritableBy (because key (from test utils) was set to allow null) // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + { + InsertOutput insertOutput = new InsertAction().execute(new InsertInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 101).withValue("firstName", "Benajamin").withValue("onlyWritableBy", null) + ))); + List errors = insertOutput.getRecords().get(0).getErrors(); + assertEquals(0, errors.size()); + } + + /////////////////////////////////////////////////////////////////////////////// + // change the null behavior to deny, and try above again, expecting an error // + /////////////////////////////////////////////////////////////////////////////// + QContext.getQInstance().getTable(TestUtils.TABLE_NAME_PERSON_MEMORY).getRecordSecurityLocks().get(0).setNullValueBehavior(RecordSecurityLock.NullValueBehavior.DENY); + { + InsertOutput insertOutput = new InsertAction().execute(new InsertInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 102).withValue("firstName", "Katherine").withValue("onlyWritableBy", null) + ))); + List errors = insertOutput.getRecords().get(0).getErrors(); + assertEquals(1, errors.size()); + assertThat(errors.get(0).getMessage()) + .contains("You do not have permission") + .contains("without a value") + .contains("Only Writable By"); + } + + } + } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateActionTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateActionTest.java index 364f1415..27878e2f 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateActionTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/UpdateActionTest.java @@ -29,12 +29,18 @@ import java.util.Objects; import com.kingsrook.qqq.backend.core.BaseTest; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput; import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateOutput; 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.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.statusmessages.QErrorMessage; +import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.TestUtils; import com.kingsrook.qqq.backend.core.utils.collections.ListBuilder; import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; @@ -492,7 +498,7 @@ class UpdateActionTest extends BaseTest updateInput.setTableName(TestUtils.TABLE_NAME_LINE_ITEM); updateInput.setRecords(List.of(new QRecord().withValue("id", 20).withValue("sku", "BASIC3"))); UpdateOutput updateOutput = new UpdateAction().execute(updateInput); - assertEquals("No record was found to update for Id = 20", updateOutput.getRecords().get(0).getErrors().get(0).getMessage()); + assertTrue(updateOutput.getRecords().get(0).getErrors().stream().anyMatch(em -> em.getMessage().equals("No record was found to update for Id = 20"))); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -504,7 +510,7 @@ class UpdateActionTest extends BaseTest updateInput.setTableName(TestUtils.TABLE_NAME_LINE_ITEM); updateInput.setRecords(List.of(new QRecord().withValue("id", 10).withValue("orderId", 2).withValue("sku", "BASIC3"))); UpdateOutput updateOutput = new UpdateAction().execute(updateInput); - assertEquals("You do not have permission to update this record - the referenced Order was not found.", updateOutput.getRecords().get(0).getErrors().get(0).getMessage()); + assertTrue(updateOutput.getRecords().get(0).getErrors().stream().anyMatch(em -> em.getMessage().equals("You do not have permission to update this record - the referenced Order was not found."))); } /////////////////////////////////////////////////////////// @@ -528,7 +534,7 @@ class UpdateActionTest extends BaseTest updateInput.setTableName(TestUtils.TABLE_NAME_LINE_ITEM_EXTRINSIC); updateInput.setRecords(List.of(new QRecord().withValue("id", 200).withValue("key", "updatedKey"))); UpdateOutput updateOutput = new UpdateAction().execute(updateInput); - assertEquals("No record was found to update for Id = 200", updateOutput.getRecords().get(0).getErrors().get(0).getMessage()); + assertTrue(updateOutput.getRecords().get(0).getErrors().stream().anyMatch(em -> em.getMessage().equals("No record was found to update for Id = 200"))); } ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -706,4 +712,79 @@ class UpdateActionTest extends BaseTest assertEquals(1, TestUtils.queryTable(TestUtils.TABLE_NAME_ORDER).stream().filter(r -> r.getValue("storeId") == null).count()); } + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testSecurityLockWriteScope() throws QException + { + TestUtils.updatePersonMemoryTableInContextWithWritableByWriteLockAndInsert3TestRecords(); + + ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // try to update them all 1, 2, and 3. 2 should be blocked, because it has a writable-By that isn't in our session // + ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + { + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("jdoe"))); + UpdateOutput updateOutput = new UpdateAction().execute(new UpdateInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 1).withValue("lastName", "Kelkhoff"), + new QRecord().withValue("id", 2).withValue("lastName", "Chamberlain"), + new QRecord().withValue("id", 3).withValue("lastName", "Maes") + ))); + + List errorRecords = updateOutput.getRecords().stream().filter(r -> CollectionUtils.nullSafeHasContents(r.getErrors())).toList(); + assertEquals(1, errorRecords.size()); + assertEquals(2, errorRecords.get(0).getValueInteger("id")); + assertThat(errorRecords.get(0).getErrors().get(0).getMessage()) + .contains("You do not have permission") + .contains("kmarsh") + .contains("Only Writable By"); + + assertEquals(2, new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_PERSON_MEMORY) + .withFilter(new QQueryFilter(new QFilterCriteria("lastName", QCriteriaOperator.IS_NOT_BLANK)))).getCount()); + } + + ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // now try to change one of the records to have a different value in the lock-field. Should fail (as it's not in our session) // + ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + { + UpdateOutput updateOutput = new UpdateAction().execute(new UpdateInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 1).withValue("onlyWritableBy", "ecartman")))); + + List errorRecords = updateOutput.getRecords().stream().filter(r -> CollectionUtils.nullSafeHasContents(r.getErrors())).toList(); + assertEquals(1, errorRecords.size()); + assertThat(errorRecords.get(0).getErrors().get(0).getMessage()) + .contains("You do not have permission") + .contains("ecartman") + .contains("Only Writable By"); + } + + /////////////////////////////////////////////////////////////// + // add that to our session and confirm we can do that update // + /////////////////////////////////////////////////////////////// + { + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("jdoe", "ecartman"))); + UpdateOutput updateOutput = new UpdateAction().execute(new UpdateInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 1).withValue("onlyWritableBy", "ecartman")))); + List errorRecords = updateOutput.getRecords().stream().filter(r -> CollectionUtils.nullSafeHasContents(r.getErrors())).toList(); + assertEquals(0, errorRecords.size()); + } + + ///////////////////////////////////////////////////////////////////////////////////////////////// + // change the null behavior to deny, then try to udpate a record and remove its onlyWritableBy // + ///////////////////////////////////////////////////////////////////////////////////////////////// + QContext.getQInstance().getTable(TestUtils.TABLE_NAME_PERSON_MEMORY).getRecordSecurityLocks().get(0).setNullValueBehavior(RecordSecurityLock.NullValueBehavior.DENY); + { + UpdateOutput updateOutput = new UpdateAction().execute(new UpdateInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 1).withValue("onlyWritableBy", null)))); + List errors = updateOutput.getRecords().get(0).getErrors(); + assertEquals(1, errors.size()); + assertThat(errors.get(0).getMessage()) + .contains("You do not have permission") + .contains("without a value") + .contains("Only Writable By"); + } + } + } diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java index a38be542..152c8d22 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/utils/TestUtils.java @@ -33,15 +33,18 @@ import com.kingsrook.qqq.backend.core.actions.dashboard.PersonsByCreateDateBarCh import com.kingsrook.qqq.backend.core.actions.processes.BackendStep; import com.kingsrook.qqq.backend.core.actions.processes.person.addtopeoplesage.AddAge; import com.kingsrook.qqq.backend.core.actions.processes.person.addtopeoplesage.GetAgeStatistics; +import com.kingsrook.qqq.backend.core.actions.tables.CountAction; import com.kingsrook.qqq.backend.core.actions.tables.InsertAction; import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.actions.tables.UpdateAction; import com.kingsrook.qqq.backend.core.actions.values.QCustomPossibleValueProvider; +import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.instances.QMetaDataVariableInterpreter; 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; +import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria; @@ -110,6 +113,9 @@ import com.kingsrook.qqq.backend.core.processes.implementations.etl.basic.BasicE import com.kingsrook.qqq.backend.core.processes.implementations.etl.streamed.StreamedETLProcess; import com.kingsrook.qqq.backend.core.processes.implementations.mock.MockBackendStep; import com.kingsrook.qqq.backend.core.processes.implementations.reports.RunReportForRecordProcess; +import com.kingsrook.qqq.backend.core.utils.collections.ListBuilder; +import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; +import static org.junit.jupiter.api.Assertions.assertEquals; /******************************************************************************* @@ -1393,4 +1399,39 @@ public class TestUtils )) ); } + + + + /******************************************************************************* + ** + *******************************************************************************/ + public static void updatePersonMemoryTableInContextWithWritableByWriteLockAndInsert3TestRecords() throws QException + { + QInstance qInstance = QContext.getQInstance(); + qInstance.addSecurityKeyType(new QSecurityKeyType() + .withName("writableBy")); + + QTableMetaData table = qInstance.getTable(TestUtils.TABLE_NAME_PERSON_MEMORY); + table.withField(new QFieldMetaData("onlyWritableBy", QFieldType.STRING).withLabel("Only Writable By")); + table.withRecordSecurityLock(new RecordSecurityLock() + .withSecurityKeyType("writableBy") + .withFieldName("onlyWritableBy") + .withNullValueBehavior(RecordSecurityLock.NullValueBehavior.ALLOW) + .withLockScope(RecordSecurityLock.LockScope.WRITE)); + + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("jdoe", "kmarsh"))); + + new InsertAction().execute(new InsertInput(TestUtils.TABLE_NAME_PERSON_MEMORY).withRecords(List.of( + new QRecord().withValue("id", 1).withValue("firstName", "Darin"), + new QRecord().withValue("id", 2).withValue("firstName", "Tim").withValue("onlyWritableBy", "kmarsh"), + new QRecord().withValue("id", 3).withValue("firstName", "James").withValue("onlyWritableBy", "jdoe") + ))); + + ////////////////////////////////////////////// + // make sure we can query for all 3 records // + ////////////////////////////////////////////// + QContext.getQSession().setSecurityKeyValues(MapBuilder.of("writableBy", ListBuilder.of("jdoe"))); + assertEquals(3, new CountAction().execute(new CountInput(TestUtils.TABLE_NAME_PERSON_MEMORY)).getCount()); + } + } 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 1301f2d5..8d223435 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 @@ -70,6 +70,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.joins.JoinType; import com.kingsrook.qqq.backend.core.model.metadata.joins.QJoinMetaData; import com.kingsrook.qqq.backend.core.model.metadata.security.QSecurityKeyType; import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLock; +import com.kingsrook.qqq.backend.core.model.metadata.security.RecordSecurityLockFilters; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.model.querystats.QueryStat; import com.kingsrook.qqq.backend.core.model.session.QSession; @@ -387,7 +388,7 @@ public abstract class AbstractRDBMSAction implements QActionInterface QQueryFilter securityFilter = new QQueryFilter(); securityFilter.setBooleanOperator(QQueryFilter.BooleanOperator.AND); - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(table.getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(table.getRecordSecurityLocks()))) { // todo - uh, if it's a RIGHT (or FULL) join, then, this should be isOuter = true, right? boolean isOuter = false; @@ -407,7 +408,7 @@ public abstract class AbstractRDBMSAction implements QActionInterface } QTableMetaData joinTable = instance.getTable(queryJoin.getJoinTable()); - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(joinTable.getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(joinTable.getRecordSecurityLocks()))) { boolean isOuter = queryJoin.getType().equals(QueryJoin.Type.LEFT); // todo full? addSubFilterForRecordSecurityLock(instance, session, joinTable, securityFilter, recordSecurityLock, joinsContext, queryJoin.getJoinTableOrItsAlias(), isOuter); @@ -1035,7 +1036,7 @@ public abstract class AbstractRDBMSAction implements QActionInterface { if(table != null) { - for(RecordSecurityLock recordSecurityLock : CollectionUtils.nonNullList(table.getRecordSecurityLocks())) + for(RecordSecurityLock recordSecurityLock : RecordSecurityLockFilters.filterForReadLocks(CollectionUtils.nonNullList(table.getRecordSecurityLocks()))) { for(String joinName : CollectionUtils.nonNullList(recordSecurityLock.getJoinNameChain())) { From 0aa0f0a0853455a4c582614cc3524a0b778ad2b7 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 17 Aug 2023 10:16:51 -0500 Subject: [PATCH 11/13] Update versions for release --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 02be6bcd..b4d7b5e3 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ - 0.18.0-SNAPSHOT + 0.18.0 UTF-8 UTF-8 From c912fe7cc2ec389289a8d2099768a88a2f4bc1f4 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 17 Aug 2023 10:16:56 -0500 Subject: [PATCH 12/13] Update for next development version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b4d7b5e3..d3ec3ca7 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ - 0.18.0 + 0.19.0-SNAPSHOT UTF-8 UTF-8 From 2e0d1dbb1c1b09c4ad081e147cc4034b00c78999 Mon Sep 17 00:00:00 2001 From: Tim Chamberlain Date: Thu, 17 Aug 2023 10:20:18 -0500 Subject: [PATCH 13/13] Updating to 0.19.0 --- qqq-dev-tools/CURRENT-SNAPSHOT-VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-dev-tools/CURRENT-SNAPSHOT-VERSION b/qqq-dev-tools/CURRENT-SNAPSHOT-VERSION index 66333910..1cf0537c 100644 --- a/qqq-dev-tools/CURRENT-SNAPSHOT-VERSION +++ b/qqq-dev-tools/CURRENT-SNAPSHOT-VERSION @@ -1 +1 @@ -0.18.0 +0.19.0