From 2422d09c314e9cd02830736bf86466d52eaf52c7 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Thu, 13 Jul 2023 09:17:07 -0500 Subject: [PATCH] Stop doing criteria expressions as their own thing, and instead put them in the values list --- .../actions/tables/query/QFilterCriteria.java | 55 ++------------- .../expressions/AbstractFilterExpression.java | 2 +- .../QFilterCriteriaDeserializer.java | 67 ++++++++++--------- .../qqq/backend/core/utils/JsonUtils.java | 38 +++++++++++ .../QFilterCriteriaDeserializerTest.java | 39 ++++++++--- .../rdbms/actions/AbstractRDBMSAction.java | 19 ++++-- .../rdbms/actions/RDBMSQueryActionTest.java | 7 +- 7 files changed, 128 insertions(+), 99 deletions(-) diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QFilterCriteria.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QFilterCriteria.java index f43f7eb9..0072b6c9 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QFilterCriteria.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/QFilterCriteria.java @@ -28,7 +28,6 @@ import java.util.Arrays; import java.util.List; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.kingsrook.qqq.backend.core.logging.QLogger; -import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.AbstractFilterExpression; import com.kingsrook.qqq.backend.core.model.actions.tables.query.serialization.QFilterCriteriaDeserializer; import com.kingsrook.qqq.backend.core.utils.CollectionUtils; import com.kingsrook.qqq.backend.core.utils.StringUtils; @@ -47,8 +46,10 @@ public class QFilterCriteria implements Serializable, Cloneable private QCriteriaOperator operator; private List values; - private String otherFieldName; - private AbstractFilterExpression expression; + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // todo - probably implement this as a type of expression - though would require a little special handling i think when evaluating... // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + private String otherFieldName; @@ -101,23 +102,6 @@ public class QFilterCriteria implements Serializable, Cloneable - /******************************************************************************* - ** - *******************************************************************************/ - public QFilterCriteria(String fieldName, QCriteriaOperator operator, AbstractFilterExpression expression) - { - this.fieldName = fieldName; - this.operator = operator; - this.expression = expression; - - /////////////////////////////////////// - // this guy doesn't like to be null? // - /////////////////////////////////////// - this.values = new ArrayList<>(); - } - - - /******************************************************************************* ** *******************************************************************************/ @@ -335,35 +319,4 @@ public class QFilterCriteria implements Serializable, Cloneable return (rs.toString()); } - - - /******************************************************************************* - ** Getter for expression - *******************************************************************************/ - public AbstractFilterExpression getExpression() - { - return (this.expression); - } - - - - /******************************************************************************* - ** Setter for expression - *******************************************************************************/ - public void setExpression(AbstractFilterExpression expression) - { - this.expression = expression; - } - - - - /******************************************************************************* - ** Fluent setter for expression - *******************************************************************************/ - public QFilterCriteria withExpression(AbstractFilterExpression expression) - { - this.expression = expression; - return (this); - } - } diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/expressions/AbstractFilterExpression.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/expressions/AbstractFilterExpression.java index 723fa818..cc58e9fa 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/expressions/AbstractFilterExpression.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/expressions/AbstractFilterExpression.java @@ -28,7 +28,7 @@ import java.io.Serializable; /******************************************************************************* ** *******************************************************************************/ -public abstract class AbstractFilterExpression +public abstract class AbstractFilterExpression implements Serializable { /******************************************************************************* ** diff --git a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializer.java b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializer.java index ae4dfd64..ca050b33 100644 --- a/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializer.java +++ b/qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializer.java @@ -23,7 +23,10 @@ package com.kingsrook.qqq.backend.core.model.actions.tables.query.serialization; import java.io.IOException; +import java.io.Serializable; import java.util.List; +import java.util.ListIterator; +import java.util.Map; import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; @@ -33,6 +36,9 @@ import com.fasterxml.jackson.databind.deser.std.StdDeserializer; 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.expressions.AbstractFilterExpression; +import com.kingsrook.qqq.backend.core.utils.CollectionUtils; +import com.kingsrook.qqq.backend.core.utils.JsonUtils; +import com.kingsrook.qqq.backend.core.utils.ValueUtils; /******************************************************************************* @@ -73,31 +79,39 @@ public class QFilterCriteriaDeserializer extends StdDeserializer values = objectMapper.treeToValue(node.get("values"), List.class); + String fieldName = objectMapper.treeToValue(node.get("fieldName"), String.class); + QCriteriaOperator operator = objectMapper.treeToValue(node.get("operator"), QCriteriaOperator.class); + String otherFieldName = objectMapper.treeToValue(node.get("otherFieldName"), String.class); - AbstractFilterExpression expression = null; - if(node.has("expression")) + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // look at all the values - if any of them are actually meant to be an Expression (instance of subclass of AbstractFilterExpression) // + // they'll have deserialized as a Map, with a "type" key. If that's the case, then re/de serialize them into the proper expression type // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + ListIterator valuesIterator = CollectionUtils.nonNullList(values).listIterator(); + while(valuesIterator.hasNext()) { - JsonNode expressionNode = node.get("expression"); - String expressionType = objectMapper.treeToValue(expressionNode.get("type"), String.class); + Object value = valuesIterator.next(); + if(value instanceof Map map && map.containsKey("type")) + { + String expressionType = ValueUtils.getValueAsString(map.get("type")); - //////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // right now, we'll assume that all expression subclasses are in the same package as AbstractFilterExpression // - // so, we can just do a Class.forName on that name, and treeToValue like that. // - // if we ever had to, we could instead switch(expressionType), and do like so... // - // case "NowWithOffset" -> objectMapper.treeToValue(expressionNode, NowWithOffset.class); // - //////////////////////////////////////////////////////////////////////////////////////////////////////////////// - try - { - String className = AbstractFilterExpression.class.getName().replace(AbstractFilterExpression.class.getSimpleName(), expressionType); - expression = (AbstractFilterExpression) objectMapper.treeToValue(expressionNode, Class.forName(className)); - } - catch(Exception e) - { - throw (new IOException("Error deserializing expression of type [" + expressionType + "] inside QFilterCriteria", e)); + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // right now, we'll assume that all expression subclasses are in the same package as AbstractFilterExpression // + // so, we can just do a Class.forName on that name, and use JsonUtils.toObject requesting that class. // + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + try + { + String assumedExpressionJSON = JsonUtils.toJson(map); + + String className = AbstractFilterExpression.class.getName().replace(AbstractFilterExpression.class.getSimpleName(), expressionType); + Serializable replacementValue = (Serializable) JsonUtils.toObject(assumedExpressionJSON, Class.forName(className)); + valuesIterator.set(replacementValue); + } + catch(Exception e) + { + throw (new IOException("Error deserializing criteria value which appeared to be an expression of type [" + expressionType + "] inside QFilterCriteria", e)); + } } } @@ -109,16 +123,7 @@ public class QFilterCriteriaDeserializer extends StdDeserializer T toObject(String json, Class targetClass) throws IOException + { + return (toObject(json, targetClass, null)); + } + + + + /******************************************************************************* + ** De-serialize a json string into an object of the specified class - with + ** customizations on the Jackson ObjectMapper. + **. + ** + ** Internally using jackson - so jackson annotations apply! + ** + *******************************************************************************/ + public static T toObject(String json, Class targetClass, Consumer objectMapperCustomizer) throws IOException { ObjectMapper objectMapper = newObjectMapper(); + if(objectMapperCustomizer != null) + { + objectMapperCustomizer.accept(objectMapper); + } return objectMapper.reader().readValue(json, targetClass); } @@ -172,6 +191,25 @@ public class JsonUtils + /******************************************************************************* + ** De-serialize a json string into an object of the specified class - with + ** customizations on the Jackson ObjectMapper. + ** + ** Internally using jackson - so jackson annotations apply! + ** + *******************************************************************************/ + public static T toObject(String json, TypeReference typeReference, Consumer objectMapperCustomizer) throws IOException + { + ObjectMapper objectMapper = newObjectMapper(); + if(objectMapperCustomizer != null) + { + objectMapperCustomizer.accept(objectMapper); + } + return objectMapper.readValue(json, typeReference); + } + + + /******************************************************************************* ** De-serialize a json string into a JSONObject (string must start with "{") ** diff --git a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializerTest.java b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializerTest.java index 42f7c3da..50f9bc8c 100644 --- a/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializerTest.java +++ b/qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/actions/tables/query/serialization/QFilterCriteriaDeserializerTest.java @@ -30,13 +30,15 @@ import com.kingsrook.qqq.backend.core.model.actions.tables.query.QFilterCriteria import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.AbstractFilterExpression; import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.Now; import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.NowWithOffset; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.ThisOrLastPeriod; import com.kingsrook.qqq.backend.core.utils.JsonUtils; 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.GREATER_THAN; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; /******************************************************************************* @@ -52,6 +54,11 @@ class QFilterCriteriaDeserializerTest extends BaseTest @Test void testDeserialize() throws IOException { + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // just put a reference to this class here, so it's a tad easier to find this class via navigation in IDE... // + /////////////////////////////////////////////////////////////////////////////////////////////////////////////// + new QFilterCriteriaDeserializer(); + { QFilterCriteria criteria = JsonUtils.toObject(""" {"fieldName": "id", "operator": "EQUALS", "values": [1]} @@ -63,14 +70,13 @@ class QFilterCriteriaDeserializerTest extends BaseTest { QFilterCriteria criteria = JsonUtils.toObject(""" - {"fieldName": "createDate", "operator": "GREATER_THAN", "expression": - {"type": "NowWithOffset", "operator": "PLUS", "amount": 5, "timeUnit": "MINUTES"} + {"fieldName": "createDate", "operator": "GREATER_THAN", "values": + [{"type": "NowWithOffset", "operator": "PLUS", "amount": 5, "timeUnit": "MINUTES"}] } """, QFilterCriteria.class); assertEquals("createDate", criteria.getFieldName()); assertEquals(GREATER_THAN, criteria.getOperator()); - assertNull(criteria.getValues()); - AbstractFilterExpression expression = criteria.getExpression(); + AbstractFilterExpression expression = (AbstractFilterExpression) criteria.getValues().get(0); assertThat(expression).isInstanceOf(NowWithOffset.class); NowWithOffset nowWithOffset = (NowWithOffset) expression; assertEquals(5, nowWithOffset.getAmount()); @@ -80,14 +86,31 @@ class QFilterCriteriaDeserializerTest extends BaseTest { QFilterCriteria criteria = JsonUtils.toObject(""" - {"fieldName": "orderDate", "operator": "EQUALS", "expression": {"type": "Now"} } + {"fieldName": "orderDate", "operator": "EQUALS", "values": [{"type": "Now"}] } """, QFilterCriteria.class); assertEquals("orderDate", criteria.getFieldName()); assertEquals(EQUALS, criteria.getOperator()); - assertNull(criteria.getValues()); - AbstractFilterExpression expression = criteria.getExpression(); + AbstractFilterExpression expression = (AbstractFilterExpression) criteria.getValues().get(0); assertThat(expression).isInstanceOf(Now.class); } + { + QFilterCriteria criteria = JsonUtils.toObject(""" + {"fieldName": "orderDate", "operator": "BETWEEN", "values": [{"type": "Now"}, {"type": "ThisOrLastPeriod"}] } + """, QFilterCriteria.class); + assertEquals("orderDate", criteria.getFieldName()); + assertEquals(BETWEEN, criteria.getOperator()); + AbstractFilterExpression expression0 = (AbstractFilterExpression) criteria.getValues().get(0); + assertThat(expression0).isInstanceOf(Now.class); + AbstractFilterExpression expression1 = (AbstractFilterExpression) criteria.getValues().get(1); + assertThat(expression1).isInstanceOf(ThisOrLastPeriod.class); + } + + { + assertThatThrownBy(() -> JsonUtils.toObject(""" + {"fieldName": "orderDate", "operator": "BETWEEN", "values": [{"type": "NotAnExpressionType"}] } + """, QFilterCriteria.class)).hasMessageContaining("Error deserializing criteria value which appeared to be an expression"); + } + } } \ No newline at end of file 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 54d8ce3c..c0890aff 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 @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -57,6 +58,7 @@ 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.QueryJoin; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.expressions.AbstractFilterExpression; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.fields.DisplayFormat; @@ -713,14 +715,23 @@ public abstract class AbstractRDBMSAction implements QActionInterface ///////////////////////////////////////////////////////////////////// values = Collections.emptyList(); } - else if(expectedNoOfParams.equals(1) && criterion.getExpression() != null) - { - values = List.of(criterion.getExpression().evaluate()); - } else if(!expectedNoOfParams.equals(values.size())) { throw new IllegalArgumentException("Incorrect number of values given for criteria [" + field.getName() + "]"); } + + ////////////////////////////////////////////////////////////// + // replace any expression-type values with their evaluation // + ////////////////////////////////////////////////////////////// + ListIterator valueListIterator = values.listIterator(); + while(valueListIterator.hasNext()) + { + Serializable value = valueListIterator.next(); + if(value instanceof AbstractFilterExpression expression) + { + valueListIterator.set(expression.evaluate()); + } + } } clauses.add("(" + clause + ")"); 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 4e49d82a..b8558e12 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 @@ -29,7 +29,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; @@ -559,7 +558,7 @@ public class RDBMSQueryActionTest extends RDBMSActionTest QueryInput queryInput = initQueryRequest(); queryInput.setFilter(new QQueryFilter() .withCriteria(new QFilterCriteria().withFieldName("lastName").withOperator(QCriteriaOperator.EQUALS).withValues(List.of("ExpressionTest"))) - .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.LESS_THAN).withExpression(new Now()))); + .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.LESS_THAN).withValues(List.of(new Now())))); QueryOutput queryOutput = new RDBMSQueryAction().execute(queryInput); assertEquals(1, queryOutput.getRecords().size(), "Expected # of rows"); Assertions.assertTrue(queryOutput.getRecords().stream().anyMatch(r -> r.getValue("firstName").equals("past")), "Should find expected row"); @@ -569,7 +568,7 @@ public class RDBMSQueryActionTest extends RDBMSActionTest QueryInput queryInput = initQueryRequest(); queryInput.setFilter(new QQueryFilter() .withCriteria(new QFilterCriteria().withFieldName("lastName").withOperator(QCriteriaOperator.EQUALS).withValues(List.of("ExpressionTest"))) - .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.LESS_THAN).withExpression(NowWithOffset.plus(2, TimeUnit.DAYS)))); + .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.LESS_THAN).withValues(List.of(NowWithOffset.plus(2, ChronoUnit.DAYS))))); QueryOutput queryOutput = new RDBMSQueryAction().execute(queryInput); assertEquals(1, queryOutput.getRecords().size(), "Expected # of rows"); Assertions.assertTrue(queryOutput.getRecords().stream().anyMatch(r -> r.getValue("firstName").equals("past")), "Should find expected row"); @@ -579,7 +578,7 @@ public class RDBMSQueryActionTest extends RDBMSActionTest QueryInput queryInput = initQueryRequest(); queryInput.setFilter(new QQueryFilter() .withCriteria(new QFilterCriteria().withFieldName("lastName").withOperator(QCriteriaOperator.EQUALS).withValues(List.of("ExpressionTest"))) - .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.GREATER_THAN).withExpression(NowWithOffset.minus(5, TimeUnit.DAYS)))); + .withCriteria(new QFilterCriteria().withFieldName("birthDate").withOperator(QCriteriaOperator.GREATER_THAN).withValues(List.of(NowWithOffset.minus(5, ChronoUnit.DAYS))))); QueryOutput queryOutput = new RDBMSQueryAction().execute(queryInput); assertEquals(2, queryOutput.getRecords().size(), "Expected # of rows"); Assertions.assertTrue(queryOutput.getRecords().stream().anyMatch(r -> r.getValue("firstName").equals("past")), "Should find expected row");