From 81248a8dafc96640b6c1e9d41ae2332a10622572 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 23 Aug 2024 09:57:08 -0500 Subject: [PATCH 1/4] CE-1646 Accept 'useCase' parameter in possibleValues function, to pass to backend, to control how possible-value filters are applied when input values are missing --- .../CriteriaMissingInputValueBehavior.java | 66 +++++ .../actions/tables/query/FilterUseCase.java | 58 +++++ .../actions/tables/query/QQueryFilter.java | 137 ++++++++++- .../PossibleValueSearchFilterUseCase.java | 72 ++++++ .../core/actions/tables/QQueryFilterTest.java | 231 ++++++++++++++++++ .../javalin/QJavalinImplementation.java | 8 +- .../javalin/QJavalinImplementationTest.java | 130 +++++++++- 7 files changed, 685 insertions(+), 17 deletions(-) create mode 100644 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/CriteriaMissingInputValueBehavior.java create mode 100644 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/FilterUseCase.java create mode 100644 qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/possiblevalues/PossibleValueSearchFilterUseCase.java diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/CriteriaMissingInputValueBehavior.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/CriteriaMissingInputValueBehavior.java new file mode 100644 index 00000000..b21d7b57 --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/CriteriaMissingInputValueBehavior.java @@ -0,0 +1,66 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.core.model.actions.tables.query; + + +/*************************************************************************** + ** Possible behaviors for doing interpretValues on a filter, and a criteria + ** has a variable value (either as a string-that-looks-like-a-variable, + ** as in ${input.foreignId} for a PVS filter, or a FilterVariableExpression), + ** and a value for that variable isn't available. + ** + ** Used in conjunction with FilterUseCase and its implementations, e.g., + ** PossibleValueSearchFilterUseCase. + ***************************************************************************/ +public enum CriteriaMissingInputValueBehavior +{ + ////////////////////////////////////////////////////////////////////// + // this was the original behavior, before we added this enum. but, // + // it doesn't ever seem entirely valid, and isn't currently used. // + ////////////////////////////////////////////////////////////////////// + INTERPRET_AS_NULL_VALUE, + + ////////////////////////////////////////////////////////////////////////// + // make the criteria behave as though it's not in the filter at all. // + // effectively by changing its operator to TRUE, so it always matches. // + // original intended use is for possible-values on query screens, // + // where a foreign-id isn't present, so we want to show all PV options. // + ////////////////////////////////////////////////////////////////////////// + REMOVE_FROM_FILTER, + + ////////////////////////////////////////////////////////////////////////////////////// + // make the criteria such that it makes no rows ever match. // + // e.g., changes it to a FALSE. I suppose, within an OR, that might // + // not be powerful enough... but, it solves the immediate use-case in // + // front of us, which is forms, where a PV field should show no values // + // until a foreign key field has a value. // + // Note that this use-case used to have the same end-effect by such // + // variables being interpreted as nulls - but this approach feels more intentional. // + ////////////////////////////////////////////////////////////////////////////////////// + MAKE_NO_MATCHES, + + /////////////////////////////////////////////////////////////////////////////////////////// + // throw an exception if a value isn't available. This is the overall default, // + // and originally was what we did for FilterVariableExpressions, e.g., for saved reports // + /////////////////////////////////////////////////////////////////////////////////////////// + THROW_EXCEPTION +} diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/FilterUseCase.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/FilterUseCase.java new file mode 100644 index 00000000..94ef53fc --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/FilterUseCase.java @@ -0,0 +1,58 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.core.model.actions.tables.query; + + +/******************************************************************************* + ** Interface where we can associate behaviors with various use cases for + ** QQueryFilters - the original being, how to handle (in the interpretValues + ** method) how to handle missing input values. + ** + ** Includes a default implementation, with a default behavior - which is to + ** throw an exception upon missing criteria variable values. + *******************************************************************************/ +public interface FilterUseCase +{ + FilterUseCase DEFAULT = new DefaultFilterUseCase(); + + /*************************************************************************** + ** + ***************************************************************************/ + CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior(); + + + /*************************************************************************** + ** + ***************************************************************************/ + class DefaultFilterUseCase implements FilterUseCase + { + + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return CriteriaMissingInputValueBehavior.THROW_EXCEPTION; + } + } +} diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QQueryFilter.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QQueryFilter.java index 6de05ddc..6006d065 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QQueryFilter.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QQueryFilter.java @@ -528,8 +528,27 @@ public class QQueryFilter implements Serializable, Cloneable ** Note - it may be very important that you call this method on a clone of a ** QQueryFilter - e.g., if it's one that defined in metaData, and that we don't ** want to be (permanently) changed!! - *******************************************************************************/ + ** + ** This overload does not take in a FilterUseCase - it uses FilterUseCase.DEFAULT + ******************************************************************************/ public void interpretValues(Map inputValues) throws QException + { + interpretValues(inputValues, FilterUseCase.DEFAULT); + } + + + + /******************************************************************************* + ** Replace any criteria values that look like ${input.XXX} with the value of XXX + ** from the supplied inputValues map - where the handling of missing values + ** is specified in the inputted FilterUseCase parameter + ** + ** Note - it may be very important that you call this method on a clone of a + ** QQueryFilter - e.g., if it's one that defined in metaData, and that we don't + ** want to be (permanently) changed!! + ** + *******************************************************************************/ + public void interpretValues(Map inputValues, FilterUseCase useCase) throws QException { List caughtExceptions = new ArrayList<>(); @@ -545,6 +564,9 @@ public class QQueryFilter implements Serializable, Cloneable { try { + Serializable interpretedValue = value; + Exception caughtException = null; + if(value instanceof AbstractFilterExpression) { /////////////////////////////////////////////////////////////////////// @@ -553,17 +575,54 @@ public class QQueryFilter implements Serializable, Cloneable /////////////////////////////////////////////////////////////////////// if(value instanceof FilterVariableExpression filterVariableExpression) { - newValues.add(filterVariableExpression.evaluateInputValues(inputValues)); - } - else - { - newValues.add(value); + try + { + interpretedValue = filterVariableExpression.evaluateInputValues(inputValues); + } + catch(Exception e) + { + caughtException = e; + interpretedValue = InputNotFound.instance; + } + } + } + else + { + ///////////////////////////////////////////////////////////////////////////////////////////////////////// + // for non-expressions, cast the value to a string, and see if it can be resolved a variable. // + // there are 3 possible cases here: // + // 1: it doesn't look like a variable, so it just comes back as a string version of whatever went in. // + // 2: it was resolved from a variable to a value, e.g., ${input.someVar} => someValue // + // 3: it looked like a variable, but no value for that variable was present in the interpreter's value // + // map - so we'll get back the InputNotFound.instance. // + ///////////////////////////////////////////////////////////////////////////////////////////////////////// + String valueAsString = ValueUtils.getValueAsString(value); + interpretedValue = variableInterpreter.interpretForObject(valueAsString, InputNotFound.instance); + } + + ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // if interpreting a value returned the not-found value, or an empty string, // + // then decide how to handle the missing value, based on the use-case input // + // Note: questionable, using "" here, but that's what reality is passing a lot for cases we want to treat as missing... // + ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + if(interpretedValue == InputNotFound.instance || "".equals(interpretedValue)) + { + CriteriaMissingInputValueBehavior missingInputValueBehavior = getMissingInputValueBehavior(useCase); + + switch(missingInputValueBehavior) + { + case REMOVE_FROM_FILTER -> criterion.setOperator(QCriteriaOperator.TRUE); + case MAKE_NO_MATCHES -> criterion.setOperator(QCriteriaOperator.FALSE); + case INTERPRET_AS_NULL_VALUE -> newValues.add(null); + + ///////////////////////////////////////////////// + // handle case in the default: THROW_EXCEPTION // + ///////////////////////////////////////////////// + default -> throw (Objects.requireNonNullElseGet(caughtException, () -> new QUserFacingException("Missing value for criteria on field: " + criterion.getFieldName()))); } } else { - String valueAsString = ValueUtils.getValueAsString(value); - Serializable interpretedValue = variableInterpreter.interpretForObject(valueAsString); newValues.add(interpretedValue); } } @@ -586,6 +645,44 @@ public class QQueryFilter implements Serializable, Cloneable + /*************************************************************************** + ** Note: in the original build of this, it felt like we *might* want to be + ** able to specify these behaviors at the individual criteria level, where + ** the implementation would be to add to QFilterCriteria: + ** - Map missingInputValueBehaviors; + ** - CriteriaMissingInputValueBehavior getMissingInputValueBehaviorForUseCase(FilterUseCase useCase) {} + * + ** (and maybe do that in a sub-class of QFilterCriteria, so it isn't always + ** there? idk...) and then here we'd call: + ** - CriteriaMissingInputValueBehavior missingInputValueBehavior = criterion.getMissingInputValueBehaviorForUseCase(useCase); + * + ** But, we don't actually have that use-case at hand now, so - let's keep it + ** just at the level we need for now. + ** + ***************************************************************************/ + private CriteriaMissingInputValueBehavior getMissingInputValueBehavior(FilterUseCase useCase) + { + if(useCase == null) + { + useCase = FilterUseCase.DEFAULT; + } + + CriteriaMissingInputValueBehavior missingInputValueBehavior = useCase.getDefaultCriteriaMissingInputValueBehavior(); + if(missingInputValueBehavior == null) + { + missingInputValueBehavior = useCase.getDefaultCriteriaMissingInputValueBehavior(); + } + + if(missingInputValueBehavior == null) + { + missingInputValueBehavior = FilterUseCase.DEFAULT.getDefaultCriteriaMissingInputValueBehavior(); + } + + return (missingInputValueBehavior); + } + + + /******************************************************************************* ** Getter for skip *******************************************************************************/ @@ -678,4 +775,28 @@ public class QQueryFilter implements Serializable, Cloneable { return Objects.hash(criteria, orderBys, booleanOperator, subFilters, skip, limit); } + + + + /*************************************************************************** + ** "Token" object to be used as the defaultIfLooksLikeVariableButNotFound + ** parameter to variableInterpreter.interpretForObject, so we can be + ** very clear that we got this default back (e.g., instead of a null, + ** which could maybe mean something else?) + ***************************************************************************/ + private static final class InputNotFound implements Serializable + { + private static InputNotFound instance = new InputNotFound(); + + + + /******************************************************************************* + ** private singleton constructor + *******************************************************************************/ + private InputNotFound() + { + + } + } + } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/possiblevalues/PossibleValueSearchFilterUseCase.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/possiblevalues/PossibleValueSearchFilterUseCase.java new file mode 100644 index 00000000..8277b067 --- /dev/null +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/possiblevalues/PossibleValueSearchFilterUseCase.java @@ -0,0 +1,72 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2024. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.kingsrook.qqq.backend.core.model.metadata.possiblevalues; + + +import com.kingsrook.qqq.backend.core.model.actions.tables.query.CriteriaMissingInputValueBehavior; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.FilterUseCase; + + +/******************************************************************************* + ** FilterUseCase implementation for the ways that possible value searches + ** are performed, and where we want to have different behaviors for criteria + ** that are missing an input value. That is, either for a: + ** + ** - FORM - e.g., creating a new record, or in a process - where we want a + ** missing filter value to basically block you from selecting a value in the + ** PVS field - e.g., you must enter some other foreign-key value before choosing + ** from this possible value - at least that's the use-case we know of now. + ** + ** - FILTER - e.g., a query screen - where there isn't really quite the same + ** scenario of choosing that foreign-key value first - so, such a PVS should + ** list all its values (e.g., a criteria missing an input value should be + ** removed from the filter). + *******************************************************************************/ +public enum PossibleValueSearchFilterUseCase implements FilterUseCase +{ + FORM(CriteriaMissingInputValueBehavior.MAKE_NO_MATCHES), + FILTER(CriteriaMissingInputValueBehavior.REMOVE_FROM_FILTER); + + + private final CriteriaMissingInputValueBehavior defaultCriteriaMissingInputValueBehavior; + + + + /*************************************************************************** + ** + ***************************************************************************/ + PossibleValueSearchFilterUseCase(CriteriaMissingInputValueBehavior defaultCriteriaMissingInputValueBehavior) + { + this.defaultCriteriaMissingInputValueBehavior = defaultCriteriaMissingInputValueBehavior; + } + + + + /******************************************************************************* + ** Getter for defaultCriteriaMissingInputValueBehavior + ** + *******************************************************************************/ + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return defaultCriteriaMissingInputValueBehavior; + } +} diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java index 1257d7e2..0ed7b54f 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java @@ -29,6 +29,8 @@ import java.util.Map; import com.kingsrook.qqq.backend.core.BaseTest; import com.kingsrook.qqq.backend.core.exceptions.QException; import com.kingsrook.qqq.backend.core.exceptions.QUserFacingException; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.CriteriaMissingInputValueBehavior; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.FilterUseCase; 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.query.expressions.AbstractFilterExpression; @@ -36,9 +38,12 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.Fil import org.junit.jupiter.api.Test; import static com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator.BETWEEN; import static com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator.EQUALS; +import static com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator.FALSE; import static com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator.IS_BLANK; +import static com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator.TRUE; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; /******************************************************************************* @@ -140,4 +145,230 @@ class QQueryFilterTest extends BaseTest assertEquals("joinTableSomeFieldIdEquals", fve7.getVariableName()); } + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testInterpretValueVariableExpressionNotFoundUseCases() throws QException + { + Map inputValues = new HashMap<>(); + + AbstractFilterExpression expression = new FilterVariableExpression() + .withVariableName("clientId"); + + //////////////////////////////////////// + // Control - where the value IS found // + //////////////////////////////////////// + inputValues.put("clientId", 47); + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + filter.interpretValues(inputValues); + assertEquals(47, filter.getCriteria().get(0).getValues().get(0)); + assertEquals(EQUALS, filter.getCriteria().get(0).getOperator()); + } + + ////////////////////////////////////////////////////// + // now - remove the value for the next set of cases // + ////////////////////////////////////////////////////// + inputValues.remove("clientId"); + + //////////////////////////////////////////////////////////////////////////////////////////////// + // a use-case that says to remove-from-filter, which, means translate to a criteria of "TRUE" // + //////////////////////////////////////////////////////////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + filter.interpretValues(inputValues, new RemoveFromFilterUseCase()); + assertEquals(0, filter.getCriteria().get(0).getValues().size()); + assertEquals(TRUE, filter.getCriteria().get(0).getOperator()); + } + + ////////////////////////////////////////////////////////////////////////////////////////////// + // a use-case that says to make-no-matches, which, means translate to a criteria of "FALSE" // + ////////////////////////////////////////////////////////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + filter.interpretValues(inputValues, new MakeNoMatchesUseCase()); + assertEquals(0, filter.getCriteria().get(0).getValues().size()); + assertEquals(FALSE, filter.getCriteria().get(0).getOperator()); + } + + /////////////////////////////////////////// + // a use-case that says to treat as null // + /////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + filter.interpretValues(inputValues, new InterpretAsNullValueUseCase()); + assertNull(filter.getCriteria().get(0).getValues().get(0)); + assertEquals(EQUALS, filter.getCriteria().get(0).getOperator()); + } + + /////////////////////////////////// + // a use-case that says to throw // + /////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + assertThatThrownBy(() -> filter.interpretValues(inputValues, new ThrowExceptionUseCase())) + .isInstanceOf(QUserFacingException.class) + .hasMessageContaining("Missing value for variable: clientId"); + } + + ////////////////////////////////////////////////////////// + // verify that empty-string is treated as not-found too // + ////////////////////////////////////////////////////////// + inputValues.put("clientId", ""); + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); + assertThatThrownBy(() -> filter.interpretValues(inputValues, new ThrowExceptionUseCase())) + .isInstanceOf(QUserFacingException.class) + .hasMessageContaining("Missing value for criteria on field: id"); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testInterpretValueStringStyleNotFoundUseCases() throws QException + { + Map inputValues = new HashMap<>(); + + //////////////////////////////////////// + // Control - where the value IS found // + //////////////////////////////////////// + inputValues.put("clientId", 47); + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + filter.interpretValues(inputValues); + assertEquals(47, filter.getCriteria().get(0).getValues().get(0)); + assertEquals(EQUALS, filter.getCriteria().get(0).getOperator()); + } + + ////////////////////////////////////////////////////// + // now - remove the value for the next set of cases // + ////////////////////////////////////////////////////// + inputValues.remove("clientId"); + + //////////////////////////////////////////////////////////////////////////////////////////////// + // a use-case that says to remove-from-filter, which, means translate to a criteria of "TRUE" // + //////////////////////////////////////////////////////////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + filter.interpretValues(inputValues, new RemoveFromFilterUseCase()); + assertEquals(0, filter.getCriteria().get(0).getValues().size()); + assertEquals(TRUE, filter.getCriteria().get(0).getOperator()); + } + + ////////////////////////////////////////////////////////////////////////////////////////////// + // a use-case that says to make-no-matches, which, means translate to a criteria of "FALSE" // + ////////////////////////////////////////////////////////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + filter.interpretValues(inputValues, new MakeNoMatchesUseCase()); + assertEquals(0, filter.getCriteria().get(0).getValues().size()); + assertEquals(FALSE, filter.getCriteria().get(0).getOperator()); + } + + /////////////////////////////////////////// + // a use-case that says to treat as null // + /////////////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + filter.interpretValues(inputValues, new InterpretAsNullValueUseCase()); + assertNull(filter.getCriteria().get(0).getValues().get(0)); + assertEquals(EQUALS, filter.getCriteria().get(0).getOperator()); + } + + /////////////////////////////////// + // a use-case that says to throw // + /////////////////////////////////// + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + assertThatThrownBy(() -> filter.interpretValues(inputValues, new ThrowExceptionUseCase())) + .isInstanceOf(QUserFacingException.class) + .hasMessageContaining("Missing value for criteria on field: id"); + } + + ////////////////////////////////////////////////////////// + // verify that empty-string is treated as not-found too // + ////////////////////////////////////////////////////////// + inputValues.put("clientId", ""); + { + QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, "${input.clientId}")); + assertThatThrownBy(() -> filter.interpretValues(inputValues, new ThrowExceptionUseCase())) + .isInstanceOf(QUserFacingException.class) + .hasMessageContaining("Missing value for criteria on field: id"); + } + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private static class RemoveFromFilterUseCase implements FilterUseCase + { + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return CriteriaMissingInputValueBehavior.REMOVE_FROM_FILTER; + } + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private static class MakeNoMatchesUseCase implements FilterUseCase + { + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return CriteriaMissingInputValueBehavior.MAKE_NO_MATCHES; + } + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private static class InterpretAsNullValueUseCase implements FilterUseCase + { + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return CriteriaMissingInputValueBehavior.INTERPRET_AS_NULL_VALUE; + } + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private static class ThrowExceptionUseCase implements FilterUseCase + { + /*************************************************************************** + ** + ***************************************************************************/ + @Override + public CriteriaMissingInputValueBehavior getDefaultCriteriaMissingInputValueBehavior() + { + return CriteriaMissingInputValueBehavior.THROW_EXCEPTION; + } + } } 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 85254753..33c2b5ad 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 @@ -114,6 +114,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.fields.FieldAdornment; 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.frontend.QFrontendVariant; +import com.kingsrook.qqq.backend.core.model.metadata.possiblevalues.PossibleValueSearchFilterUseCase; import com.kingsrook.qqq.backend.core.model.metadata.possiblevalues.QPossibleValueSource; import com.kingsrook.qqq.backend.core.model.metadata.processes.QProcessMetaData; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; @@ -125,6 +126,7 @@ import com.kingsrook.qqq.backend.core.modules.authentication.implementations.Aut import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.ExceptionUtils; import com.kingsrook.qqq.backend.core.utils.JsonUtils; +import com.kingsrook.qqq.backend.core.utils.ObjectUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; import com.kingsrook.qqq.backend.core.utils.ValueUtils; import com.kingsrook.qqq.backend.core.utils.collections.MapBuilder; @@ -1792,7 +1794,11 @@ public class QJavalinImplementation } defaultQueryFilter = field.getPossibleValueSourceFilter().clone(); - defaultQueryFilter.interpretValues(values); + + String useCaseParam = QJavalinUtils.getQueryParamOrFormParam(context, "useCase"); + PossibleValueSearchFilterUseCase useCase = ObjectUtils.tryElse(() -> PossibleValueSearchFilterUseCase.valueOf(useCaseParam.toUpperCase()), PossibleValueSearchFilterUseCase.FORM); + + defaultQueryFilter.interpretValues(values, useCase); } finishPossibleValuesRequest(context, field.getPossibleValueSourceName(), defaultQueryFilter); diff --git a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java index 1a61710c..9f59d373 100644 --- a/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java +++ b/qqq-middleware-javalin/src/test/java/com/kingsrook/qqq/backend/javalin/QJavalinImplementationTest.java @@ -950,24 +950,138 @@ class QJavalinImplementationTest extends QJavalinTestBase + /*************************************************************************** + ** + ***************************************************************************/ + private JSONArray assertPossibleValueSuccessfulResponseAndGetOptionsArray(HttpResponse response) + { + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertNotNull(jsonObject); + return (jsonObject.getJSONArray("options")); + } + + + + /*************************************************************************** + ** + ***************************************************************************/ + private void assertPossibleValueSuccessfulResponseWithNoOptions(HttpResponse response) + { + assertEquals(200, response.getStatus()); + JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); + assertNotNull(jsonObject); + assertFalse(jsonObject.has("options")); // no results comes back as result w/o options array. + } + + + /******************************************************************************* ** *******************************************************************************/ @Test void testPossibleValueWithFilter() { + ///////////////////////////////////////////////////////////// + // post with no search term, and values that find a result // + ///////////////////////////////////////////////////////////// HttpResponse response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=") .field("values", """ - {"email":"tsamples@mmltholdings.com"} - """) + {"email":"tsamples@mmltholdings.com"} + """) .asString(); - assertEquals(200, response.getStatus()); - JSONObject jsonObject = JsonUtils.toJSONObject(response.getBody()); - assertNotNull(jsonObject); - assertNotNull(jsonObject.getJSONArray("options")); - assertEquals(1, jsonObject.getJSONArray("options").length()); - assertEquals("Tyler Samples (4)", jsonObject.getJSONArray("options").getJSONObject(0).getString("label")); + JSONArray options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + assertNotNull(options); + assertEquals(1, options.length()); + assertEquals("Tyler Samples (4)", options.getJSONObject(0).getString("label")); + + /////////////////////////////////////////////////////////// + // post with search term and values that find no results // + /////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=notFound") + .field("values", """ + {"email":"tsamples@mmltholdings.com"} + """) + .asString(); + assertPossibleValueSuccessfulResponseWithNoOptions(response); + + //////////////////////////////////////////////////////////////// + // post with no search term, but values that cause no matches // + //////////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=") + .field("values", """ + {"email":"noUser@mmltholdings.com"} + """) + .asString(); + assertPossibleValueSuccessfulResponseWithNoOptions(response); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testPossibleValueWithFilterMissingValue() + { + ///////////////////////////////////////////////////////////// + // filter use-case, with no values, should return options. // + ///////////////////////////////////////////////////////////// + HttpResponse response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=filter").asString(); + JSONArray options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + assertNotNull(options); + assertThat(options.length()).isGreaterThanOrEqualTo(5); + + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // similarly, values map, but not the 'email' value, that this PVS field is based on, should return options. // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=filter") + .field("values", """ + {"userId":"123"} + """) + .asString(); + options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + assertNotNull(options); + assertThat(options.length()).isGreaterThanOrEqualTo(5); + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // similarly, values map, with the email value, but an empty string in there - should act the same as if it's missing, and not filter the values. // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=filter") + .field("values", """ + {"email":""} + """) + .asString(); + options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + assertNotNull(options); + assertThat(options.length()).isGreaterThanOrEqualTo(5); + + ///////////////////////////////////////////////////////////////////////// + // versus form use-case with no values, which should return 0 options. // + ///////////////////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=form").asString(); + assertPossibleValueSuccessfulResponseWithNoOptions(response); + + ///////////////////////////////////////////////////////////////////////////////// + // versus form use-case with expected value, which should return some options. // + ///////////////////////////////////////////////////////////////////////////////// + response = Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=form") + .field("values", """ + {"email":"tsamples@mmltholdings.com"} + """) + .asString(); + options = assertPossibleValueSuccessfulResponseAndGetOptionsArray(response); + assertNotNull(options); + assertEquals(1, options.length()); + assertEquals("Tyler Samples (4)", options.getJSONObject(0).getString("label")); + + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // finally an unrecognized useCase (or missing or empty), should behave the same as a form, and return 0 options if the filter-value is missing. // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + assertPossibleValueSuccessfulResponseWithNoOptions(Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=notAUseCase").asString()); + assertPossibleValueSuccessfulResponseWithNoOptions(Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=").asString()); + assertPossibleValueSuccessfulResponseWithNoOptions(Unirest.post(BASE_URL + "/data/pet/possibleValues/ownerPersonId?searchTerm=&useCase=").asString()); } From ed1e2519342df18b75ff98075793b2877f0e9bd6 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 23 Aug 2024 10:01:20 -0500 Subject: [PATCH 2/4] CE-1646 Fix expected message on one test --- .../qqq/backend/core/actions/tables/QQueryFilterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java index 0ed7b54f..2e64a870 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/tables/QQueryFilterTest.java @@ -222,7 +222,7 @@ class QQueryFilterTest extends BaseTest QQueryFilter filter = new QQueryFilter(new QFilterCriteria("id", EQUALS, expression)); assertThatThrownBy(() -> filter.interpretValues(inputValues, new ThrowExceptionUseCase())) .isInstanceOf(QUserFacingException.class) - .hasMessageContaining("Missing value for criteria on field: id"); + .hasMessageContaining("Missing value for variable: clientId"); } } From 89e0fc566d49712c34c9ac937d9cfc37d5e71df5 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 23 Aug 2024 12:16:44 -0500 Subject: [PATCH 3/4] Try to fix flaky test --- .../C3P0PooledConnectionProviderTest.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/jdbc/C3P0PooledConnectionProviderTest.java b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/jdbc/C3P0PooledConnectionProviderTest.java index 417fd6cc..c588a06a 100644 --- a/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/jdbc/C3P0PooledConnectionProviderTest.java +++ b/qqq-backend-module-rdbms/src/test/java/com/kingsrook/qqq/backend/module/rdbms/jdbc/C3P0PooledConnectionProviderTest.java @@ -102,6 +102,14 @@ class C3P0PooledConnectionProviderTest extends BaseTest backend.setConnectionProvider(new QCodeReference(C3P0PooledConnectionProvider.class)); QContext.init(qInstance, new QSession()); + ///////////////////////////////////////////////////////////////////////////////// + // sometimes we're seeing this test fail w/ only 2 connections in the pool... // + // theory is, maybe, the pool doesn't quite have enough time to open them all? // + // so, try adding a little sleep here. // + ///////////////////////////////////////////////////////////////////////////////// + new QueryAction().execute(new QueryInput(TestUtils.TABLE_NAME_PERSON)); + SleepUtils.sleep(500, TimeUnit.MILLISECONDS); + for(int i = 0; i < 5; i++) { new QueryAction().execute(new QueryInput(TestUtils.TABLE_NAME_PERSON)); @@ -110,17 +118,18 @@ class C3P0PooledConnectionProviderTest extends BaseTest JSONObject debugValues = getDebugStateValues(true); assertThat(debugValues.getInt("numConnections")).isBetween(3, 6); // due to potential timing issues, sometimes pool will acquire another 3 conns, so 3 or 6 seems ok. - //////////////////////////////////////////////////////////////////// - // open up 4 transactions - confirm the pool opens some new conns // - //////////////////////////////////////////////////////////////////// + ////////////////////////////////////////////////////////////////////////// + // open up several transactions - confirm the pool opens some new conns // + ////////////////////////////////////////////////////////////////////////// + int noTransactions = 7; List transactions = new ArrayList<>(); - for(int i = 0; i < 5; i++) + for(int i = 0; i < noTransactions; i++) { transactions.add(QBackendTransaction.openFor(new InsertInput(TestUtils.TABLE_NAME_PERSON))); } debugValues = getDebugStateValues(true); - assertThat(debugValues.getInt("numConnections")).isGreaterThan(3); + assertThat(debugValues.getInt("numConnections")).isGreaterThanOrEqualTo(noTransactions); transactions.forEach(transaction -> transaction.close()); @@ -128,7 +137,7 @@ class C3P0PooledConnectionProviderTest extends BaseTest // might take a second for the pool to re-claim the closed connections // ///////////////////////////////////////////////////////////////////////// boolean foundMatch = false; - for(int i = 0; i < 5; i++) + for(int i = 0; i < noTransactions; i++) { debugValues = getDebugStateValues(true); if(debugValues.getInt("numConnections") == debugValues.getInt("numIdleConnections")) From 666f4a872dcf3393732b5737ae757e73ed45d203 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Fri, 23 Aug 2024 14:36:23 -0500 Subject: [PATCH 4/4] CE-1646 add use-cases to preserve the previous behavior for whether a report w/ missing input criteria values should fail or not --- .../reporting/GenerateReportAction.java | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/reporting/GenerateReportAction.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/reporting/GenerateReportAction.java index eff97f4a..8acdf977 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/reporting/GenerateReportAction.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/reporting/GenerateReportAction.java @@ -63,6 +63,8 @@ import com.kingsrook.qqq.backend.core.model.actions.reporting.ReportOutput; import com.kingsrook.qqq.backend.core.model.actions.tables.QueryHint; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountOutput; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.CriteriaMissingInputValueBehavior; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.FilterUseCase; import com.kingsrook.qqq.backend.core.model.actions.tables.query.JoinsContext; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterOrderBy; import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; @@ -614,7 +616,56 @@ public class GenerateReportAction extends AbstractQActionFunction