From cd2ac841164e6e371251c96ddfb5ca6db43f67da Mon Sep 17 00:00:00 2001 From: David Stephan Date: Wed, 28 Jan 2026 11:06:12 +0100 Subject: [PATCH 1/2] SED-4488 Poor performance of the steps table when filtering by statuses --- .../mongodb/MongoDBFilterFactory.java | 11 +++- .../postgresql/PostgreSQLCollection.java | 4 +- .../postgresql/PostgreSQLFilterFactory.java | 19 +++++++ .../step/core/accessors/AbstractAccessor.java | 3 +- .../step/core/collections/PojoFilters.java | 44 +++++++++++++++ .../java/step/core/ql/OQLFilterVisitor.java | 2 +- .../collections/AbstractCollectionTest.java | 15 +++++ .../OQLAttributesAwareFilterVisitorTest.java | 13 +++-- .../java/step/core/collections/Filter.java | 3 +- .../java/step/core/collections/Filters.java | 12 ++-- .../step/core/collections/filters/In.java | 55 +++++++++++++++++++ 11 files changed, 160 insertions(+), 21 deletions(-) create mode 100644 step-framework-model/src/main/java/step/core/collections/filters/In.java diff --git a/step-framework-collections-mongodb/src/main/java/step/core/collections/mongodb/MongoDBFilterFactory.java b/step-framework-collections-mongodb/src/main/java/step/core/collections/mongodb/MongoDBFilterFactory.java index 93a216f0..d8ca75dd 100644 --- a/step-framework-collections-mongodb/src/main/java/step/core/collections/mongodb/MongoDBFilterFactory.java +++ b/step-framework-collections-mongodb/src/main/java/step/core/collections/mongodb/MongoDBFilterFactory.java @@ -83,7 +83,16 @@ public Bson buildFilter(Filter filter) { } else if (filter instanceof Exists) { Exists existsFilter = (Exists) filter; return com.mongodb.client.model.Filters.exists(existsFilter.getField()); - } else { + } else if (filter instanceof In) { + In inFilter = (In) filter; + String field = inFilter.getField(); + List values = inFilter.getValues(); + if(field.equals("id")) { + field = "_id"; + values = values.stream().map(s -> (s instanceof String) ? new ObjectId((String) s) : s).collect(Collectors.toList()); + } + return com.mongodb.client.model.Filters.in(field, values); + }else { throw new IllegalArgumentException("Unsupported filter type " + filter.getClass()); } } diff --git a/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLCollection.java b/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLCollection.java index ad10e40d..c3dbc4d8 100644 --- a/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLCollection.java +++ b/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLCollection.java @@ -106,8 +106,8 @@ public long count(Filter filter, Integer limit) { String query = "SELECT count(d.*) FROM (SELECT id FROM " + collectionNameStr + " WHERE " + filterToWhereClause(filter) + " LIMIT " + limit + ") d"; try (Connection connection = ds.getConnection(); - Statement statement = connection.createStatement()) { - ResultSet resultSet = statement.executeQuery(query); + Statement statement = connection.createStatement(); + ResultSet resultSet = statement.executeQuery(query)) { if (resultSet.next()) { return resultSet.getInt(1); } else { diff --git a/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLFilterFactory.java b/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLFilterFactory.java index 4356b70a..94ea6ee2 100644 --- a/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLFilterFactory.java +++ b/step-framework-collections-postgresql/src/main/java/step/core/collections/postgresql/PostgreSQLFilterFactory.java @@ -91,6 +91,13 @@ public String buildFilter(Filter filter) { } else if (filter instanceof Exists) { Exists existsFilter = (Exists) filter; return formatField(existsFilter.getField(),false) + " IS NOT NULL "; + } else if (filter instanceof In) { + In inFilter = (In) filter; + //In filter values implementation only support comparison as Strings, so the field and values are formated to string + String values = inFilter.getValues().stream() + .map(this::formatInValue) // Escape single quotes for SQL + .collect(Collectors.joining(",", "(", ")")); + return formatField(inFilter.getField(), true) + " IN " + values + " "; } else { throw new IllegalArgumentException("Unsupported filter type " + filter.getClass()); } @@ -106,6 +113,18 @@ private String notPsqlClause(Not notFilter, List childerPojoFilters) { } } + private String formatInValue(Object expectedValue) { + String result; + if (expectedValue instanceof String) { + result = escapeValue((String) expectedValue); + } else if (expectedValue instanceof ObjectId) { + result = ((ObjectId) expectedValue).toHexString(); + } else { + result = expectedValue.toString(); + } + return "'" + result + "'"; + } + private String escapeValue(String expectedValue) { return expectedValue.replaceAll("'","''"); } diff --git a/step-framework-collections/src/main/java/step/core/accessors/AbstractAccessor.java b/step-framework-collections/src/main/java/step/core/accessors/AbstractAccessor.java index 73f3b695..ab49ac2f 100644 --- a/step-framework-collections/src/main/java/step/core/accessors/AbstractAccessor.java +++ b/step-framework-collections/src/main/java/step/core/accessors/AbstractAccessor.java @@ -76,9 +76,8 @@ public T findByCriteria(Map criteria) { @Override public Stream findByIds(List ids) { - List idsList = ids.stream() + List idsList = ids.stream() .map(ObjectId::new) - .map(ObjectId::toString) .collect(Collectors.toList()); return collectionDriver.find(Filters.in("id", idsList), null, null, null, 0); } diff --git a/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java b/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java index b3ef7a48..8ce23117 100644 --- a/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java +++ b/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java @@ -26,6 +26,7 @@ import java.lang.reflect.InvocationTargetException; import java.math.BigDecimal; import java.util.List; +import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -76,6 +77,8 @@ public PojoFilter buildFilter(Filter filter) { return new GtePojoFilter<>((Gte) filter); } else if (filter instanceof Exists) { return new ExistsPojoFilter<>((Exists) filter); + } else if (filter instanceof In) { + return new InPojoFilter<>((In) filter); } else { throw new IllegalArgumentException("Unsupported filter type " + filter.getClass()); } @@ -356,6 +359,47 @@ public boolean test(T t) { return beanProperty != null; } } + + public static class InPojoFilter implements PojoFilter { + + private final String field; + private final List values; + + public InPojoFilter(In inFilter) { + super(); + this.field = inFilter.getField(); + if (field.equals(AbstractIdentifiableObject.ID)) { + //Convert String values to ObjectId as for the equal filter + values = inFilter.getValues().stream().map(v -> (v instanceof String) ? new ObjectId((String) v) : v).collect(Collectors.toList()); + } else { + values = inFilter.getValues(); + } + } + + @Override + public boolean test(T t) { + try { + Object beanProperty = getBeanProperty(t, field); + if(beanProperty != null) { + return values.stream().anyMatch(v -> v != null && compareNonNullValues(beanProperty, v)); + } else { + return values.stream().anyMatch(Objects::isNull); + } + } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + return false; + } + } + + public boolean compareNonNullValues(Object beanValue, Object inValue) { + if (beanValue instanceof ObjectId && inValue instanceof String) { + return ((ObjectId) beanValue).toHexString().equals(inValue); + } else if (beanValue instanceof String && inValue instanceof ObjectId) { + return ((ObjectId) inValue).toHexString().equals(beanValue); + } else { + return beanValue.equals(inValue); + } + } + } private static Object getBeanProperty(Object t, String fieldName) throws IllegalAccessException, InvocationTargetException, NoSuchMethodException { diff --git a/step-framework-collections/src/main/java/step/core/ql/OQLFilterVisitor.java b/step-framework-collections/src/main/java/step/core/ql/OQLFilterVisitor.java index 87cebaa5..b1067972 100644 --- a/step-framework-collections/src/main/java/step/core/ql/OQLFilterVisitor.java +++ b/step-framework-collections/src/main/java/step/core/ql/OQLFilterVisitor.java @@ -106,7 +106,7 @@ public Filter visitInExpr(InExprContext ctx) { } protected Filter processInExpr(InExprContext ctx, String text0) { - List ins = ctx.STRING().stream().map(tn -> unescapeStringIfNecessary(tn.getText())).collect(Collectors.toList()); + List ins = ctx.STRING().stream().map(tn -> unescapeStringIfNecessary(tn.getText())).collect(Collectors.toList()); return Filters.in(text0, ins); } diff --git a/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java b/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java index 9c0a1710..ef4da0d5 100644 --- a/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java +++ b/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java @@ -301,6 +301,7 @@ public void testFindFilters() { result = collection.find(Filters.equals("missingField", (String) null), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(bean.getId(), result.get(0).getId()); + //and result = collection.find(Filters.and(List.of(Filters.gte("longProperty", 11),Filters.lt("longProperty",21))), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(bean.getId(), result.get(0).getId()); assertEquals(1, result.size()); @@ -309,6 +310,18 @@ public void testFindFilters() { assertEquals(bean2.getId(), result.get(0).getId()); assertEquals(1, result.size()); + //or + result = collection.find(Filters.or(List.of(Filters.equals("property1", "My property 1"),Filters.equals("property1", "My property 2"))), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(2, result.size()); + + //in + result = collection.find(Filters.in("property1", List.of("My property 1")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); + assertEquals(bean.getId(), result.get(0).getId()); + + result = collection.find(Filters.in("property1", List.of("My property 1", "My property 2")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(2, result.size()); + //Not equal result = collection.find(Filters.not(Filters.equals("attributes.MyAtt2", "My value 1")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(2, result.size()); @@ -328,6 +341,8 @@ public void testFindFilters() { assertThrows(Throwable.class, () -> collection.find(Filters.and(new ArrayList<>()), null, null, null, 0).collect(Collectors.toList())); assertThrows(Throwable.class, () -> collection.find(Filters.or(new ArrayList<>()), null, null, null, 0).collect(Collectors.toList())); + + assertThrows(Throwable.class, () -> collection.find(Filters.in("test", new ArrayList<>()), null, null, null, 0).collect(Collectors.toList())); } @Test diff --git a/step-framework-collections/src/test/java/step/core/ql/OQLAttributesAwareFilterVisitorTest.java b/step-framework-collections/src/test/java/step/core/ql/OQLAttributesAwareFilterVisitorTest.java index ad42f227..598a3cb1 100644 --- a/step-framework-collections/src/test/java/step/core/ql/OQLAttributesAwareFilterVisitorTest.java +++ b/step-framework-collections/src/test/java/step/core/ql/OQLAttributesAwareFilterVisitorTest.java @@ -5,7 +5,7 @@ import org.junit.Test; import step.core.collections.Filter; import step.core.collections.filters.Equals; -import step.core.collections.filters.Or; +import step.core.collections.filters.In; import step.core.collections.filters.True; import java.util.Arrays; @@ -66,11 +66,12 @@ public void transformWithInCriteriaTest() { OQLParser.ParseContext context = parse("field1 in ( \"prop1\", \"prop2\" )"); OQLAttributesAwareFilterVisitor visitor = new OQLAttributesAwareFilterVisitor((key) -> "prefix." + key); Filter filter = visitor.visit(context.getChild(0)); - Assert.assertTrue(filter instanceof Or); - Or orFilter = (Or) filter; - Assert.assertEquals(2, orFilter.getChildren().size()); - Equals prop1Equals = (Equals) orFilter.getChildren().get(0); - Assert.assertEquals("prefix.field1", prop1Equals.getField()); + Assert.assertTrue(filter instanceof In); + In inFilter = (In) filter; + Assert.assertEquals(2, inFilter.getValues().size()); + Assert.assertEquals("prefix.field1", inFilter.getField()); + Assert.assertEquals("prop1", inFilter.getValues().get(0)); + Assert.assertEquals("prop2", inFilter.getValues().get(1)); } private static OQLParser.ParseContext parse(String expression) { diff --git a/step-framework-model/src/main/java/step/core/collections/Filter.java b/step-framework-model/src/main/java/step/core/collections/Filter.java index d7d5fd92..a47310db 100644 --- a/step-framework-model/src/main/java/step/core/collections/Filter.java +++ b/step-framework-model/src/main/java/step/core/collections/Filter.java @@ -40,7 +40,8 @@ @JsonSubTypes.Type(Or.class), @JsonSubTypes.Type(Regex.class), @JsonSubTypes.Type(True.class), - @JsonSubTypes.Type(Exists.class) + @JsonSubTypes.Type(Exists.class), + @JsonSubTypes.Type(In.class) }) public interface Filter { diff --git a/step-framework-model/src/main/java/step/core/collections/Filters.java b/step-framework-model/src/main/java/step/core/collections/Filters.java index 3203dc83..cd1217b2 100644 --- a/step-framework-model/src/main/java/step/core/collections/Filters.java +++ b/step-framework-model/src/main/java/step/core/collections/Filters.java @@ -95,14 +95,6 @@ public static Gte gte(String field, long value) { return new Gte(field, value); } - public static Filter in(String field, List values) { - List filters = new ArrayList<>(); - for (String v : values) { - filters.add(Filters.equals(field, v)); - } - return (filters.isEmpty()) ? Filters.falseFilter() : Filters.or(filters); - } - public static Regex regex(String field, String expression, boolean caseSensitive) { return new Regex(field, expression, caseSensitive); } @@ -115,6 +107,10 @@ public static Exists exists(String field) { return new Exists(field); } + public static In in(String field, List values) { + return new In(field, values); + } + /** * Helper method to extract all attributes (fields) on which the filters are applied * @param filter the filter for which with extract the list of attributes diff --git a/step-framework-model/src/main/java/step/core/collections/filters/In.java b/step-framework-model/src/main/java/step/core/collections/filters/In.java new file mode 100644 index 00000000..de360c81 --- /dev/null +++ b/step-framework-model/src/main/java/step/core/collections/filters/In.java @@ -0,0 +1,55 @@ +package step.core.collections.filters; + +import java.util.List; +import java.util.Objects; + +public class In extends AbstractAtomicFilter { + + private String field; + private List values; + + public In() { + super(); + } + + public In(String field, List values) { + super(); + Objects.requireNonNull(field, "The field of the In filter cannot be null."); + Objects.requireNonNull(values, "The list of values of the In filter cannot be null."); + if (values.isEmpty()) { + throw new IllegalArgumentException("The list of values of the In filter cannot be empty."); + } + this.field = field; + this.values = values; + } + + @Override + public String getField() { + return field; + } + + public List getValues() { + return values; + } + + public void setField(String field) { + this.field = field; + } + + public void setValues(List values) { + this.values = values; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + In in = (In) o; + return values == in.values && Objects.equals(field, in.field); + } + + @Override + public int hashCode() { + return Objects.hash(field, values); + } +} From c843def9d61d96c25bc578b61cfe672ccb285143 Mon Sep 17 00:00:00 2001 From: David Stephan Date: Thu, 29 Jan 2026 09:37:16 +0100 Subject: [PATCH 2/2] SED-4488 PR feedbacks --- .../step/core/collections/PojoFilters.java | 51 +++++++++---------- .../collections/AbstractCollectionTest.java | 30 ++++++++++- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java b/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java index 8ce23117..0fc07b6f 100644 --- a/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java +++ b/step-framework-collections/src/main/java/step/core/collections/PojoFilters.java @@ -176,27 +176,31 @@ public boolean test(T t) { try { String field = equalsFilter.getField(); Object beanProperty = getBeanProperty(t, field); - if(expectedValue != null) { - if(expectedValue instanceof Number) { - if(beanProperty != null) { - return new BigDecimal(expectedValue.toString()).compareTo(new BigDecimal(beanProperty.toString()))==0; - } else { - return false; - } - } if (expectedValue instanceof String && beanProperty!= null && beanProperty.getClass().isEnum()) { - return expectedValue.equals(beanProperty.toString()); - } else { - return expectedValue.equals(beanProperty); - } - } else { - return beanProperty == null; - } + return testProperty(beanProperty); } catch (NoSuchMethodException e) { return (expectedValue == null); } catch (IllegalAccessException | InvocationTargetException e) { return false; } } + + public boolean testProperty(Object beanProperty) { + if(expectedValue != null) { + if(expectedValue instanceof Number) { + if(beanProperty instanceof Number) { + return new BigDecimal(expectedValue.toString()).compareTo(new BigDecimal(beanProperty.toString()))==0; + } else { + return false; + } + } if (expectedValue instanceof String && beanProperty!= null && beanProperty.getClass().isEnum()) { + return expectedValue.equals(beanProperty.toString()); + } else { + return expectedValue.equals(beanProperty); + } + } else { + return beanProperty == null; + } + } } public static class RegexPojoFilter implements PojoFilter { @@ -363,28 +367,19 @@ public boolean test(T t) { public static class InPojoFilter implements PojoFilter { private final String field; - private final List values; + private final List> equalFilters; public InPojoFilter(In inFilter) { super(); - this.field = inFilter.getField(); - if (field.equals(AbstractIdentifiableObject.ID)) { - //Convert String values to ObjectId as for the equal filter - values = inFilter.getValues().stream().map(v -> (v instanceof String) ? new ObjectId((String) v) : v).collect(Collectors.toList()); - } else { - values = inFilter.getValues(); - } + field = inFilter.getField(); + equalFilters = inFilter.getValues().stream().map(v -> new EqualsPojoFilter(new Equals(field, v))).collect(Collectors.toList()); } @Override public boolean test(T t) { try { Object beanProperty = getBeanProperty(t, field); - if(beanProperty != null) { - return values.stream().anyMatch(v -> v != null && compareNonNullValues(beanProperty, v)); - } else { - return values.stream().anyMatch(Objects::isNull); - } + return equalFilters.stream().anyMatch(eq -> eq.testProperty(beanProperty)); } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { return false; } diff --git a/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java b/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java index ef4da0d5..ad9fd2f5 100644 --- a/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java +++ b/step-framework-collections/src/test/java/step/core/collections/AbstractCollectionTest.java @@ -268,7 +268,7 @@ public void testFindFilters() { Bean bean2 = new Bean(); bean2.setProperty1("My property 2"); bean2.setLongProperty(21l); - bean2.setBooleanProperty(false); + bean2.setBooleanProperty(true); bean2.addAttribute("MyAtt1", "My value 2"); bean2.addAttribute("MyAtt2", "My other value"); collection.save(bean2); @@ -291,12 +291,16 @@ public void testFindFilters() { // Equals boolean result = collection.find(Filters.equals("booleanProperty", false), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); assertEquals(bean.getId(), result.get(0).getId()); // Equals long result = collection.find(Filters.equals("longProperty", 11), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(bean.getId(), result.get(0).getId()); + result = collection.find(Filters.equals("longProperty", 11L), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(bean.getId(), result.get(0).getId()); + //is null result = collection.find(Filters.equals("missingField", (String) null), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(bean.getId(), result.get(0).getId()); @@ -321,6 +325,30 @@ public void testFindFilters() { result = collection.find(Filters.in("property1", List.of("My property 1", "My property 2")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); assertEquals(2, result.size()); + assertEquals(bean.getId(), result.get(0).getId()); + + //in boolean + mix in values + result = collection.find(Filters.in("booleanProperty", List.of("My property 1", true, 12)), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); + assertEquals(bean2.getId(), result.get(0).getId()); + + //in long + mix in values + result = collection.find(Filters.in("longProperty", List.of("My property 1", true, 21)), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); + assertEquals(bean2.getId(), result.get(0).getId()); + + result = collection.find(Filters.in("property1", List.of("My property 1")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); + assertEquals(bean.getId(), result.get(0).getId()); + + //in IDs + result = collection.find(Filters.in("id", List.of(bean.getId(), bean2.getId())), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(2, result.size()); + assertEquals(bean.getId(), result.get(0).getId()); + + result = collection.find(Filters.in("id", List.of(bean.getId().toHexString())), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList()); + assertEquals(1, result.size()); + assertEquals(bean.getId(), result.get(0).getId()); //Not equal result = collection.find(Filters.not(Filters.equals("attributes.MyAtt2", "My value 1")), new SearchOrder("MyAtt1", 1), null, null, 0).collect(Collectors.toList());