From d4e5bff5515634d1057959a071f1bab17a68802d Mon Sep 17 00:00:00 2001 From: Suresh Prakash <93120060+suresh-prakash@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:57:55 +0530 Subject: [PATCH] Introduce NOT logical operator and add tests (#192) --- .../ArrayFiltersQueryIntegrationTest.java | 95 ++++++++++++++++++- ..._planets_having_both_oxygen_and_water.json | 31 ++++++ ...having_both_oxygen_and_water_together.json | 56 +++++++++++ .../expression/impl/LogicalExpression.java | 19 +++- .../expression/operators/LogicalOperator.java | 1 + .../config/mongo/MongoConnectionConfig.java | 2 +- .../parser/MongoLogicalExpressionParser.java | 13 ++- .../PostgresFilterTypeExpressionVisitor.java | 5 + .../config/MongoConnectionConfigTest.java | 8 +- 9 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 document-store/src/integrationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json create mode 100644 document-store/src/integrationTest/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json diff --git a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java index 7cc517c6..3fe6c73d 100644 --- a/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java +++ b/document-store/src/integrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore; import static org.hypertrace.core.documentstore.expression.impl.LogicalExpression.and; +import static org.hypertrace.core.documentstore.expression.impl.LogicalExpression.not; import static org.hypertrace.core.documentstore.expression.operators.ArrayOperator.ANY; import static org.hypertrace.core.documentstore.model.config.DatabaseType.MONGO; import static org.hypertrace.core.documentstore.model.config.DatabaseType.POSTGRES; @@ -161,7 +162,7 @@ public Stream provideArguments(final ExtensionContext context) { @ParameterizedTest @ArgumentsSource(AllProvider.class) - void getAllSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String dataStoreName) + void getSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String dataStoreName) throws JSONException { final Collection collection = getCollection(dataStoreName); @@ -203,6 +204,98 @@ void getAllSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT); } + @ParameterizedTest + @ArgumentsSource(AllProvider.class) + void getSolarSystemsWithAllThePlanetsHavingBothWaterAndOxygen(final String dataStoreName) + throws JSONException { + final Collection collection = getCollection(dataStoreName); + + // All planets having both oxygen and water = No planet without both oxygen and water + final Query query = + Query.builder() + .setFilter( + not( + DocumentArrayFilterExpression.builder() + .operator(ANY) + .arraySource(IdentifierExpression.of("additional_info.planets")) + .filter( + not( + and( + ArrayRelationalFilterExpression.builder() + .operator(ANY) + .filter( + RelationalExpression.of( + IdentifierExpression.of("elements"), + RelationalOperator.EQ, + ConstantExpression.of("Oxygen"))) + .build(), + ArrayRelationalFilterExpression.builder() + .operator(ANY) + .filter( + RelationalExpression.of( + IdentifierExpression.of("elements"), + RelationalOperator.EQ, + ConstantExpression.of("Water"))) + .build(), + RelationalExpression.of( + IdentifierExpression.of("meta.num_moons"), + RelationalOperator.GT, + ConstantExpression.of(0))))) + .build())) + .build(); + + final Iterator documents = collection.aggregate(query); + final String expected = readResource("all_planets_having_both_oxygen_and_water.json"); + final String actual = iteratorToJson(documents); + + JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT); + } + + @ParameterizedTest + @ArgumentsSource(AllProvider.class) + void getSolarSystemsWithNoneOfThePlanetsHavingBothWaterAndOxygenTogether( + final String dataStoreName) throws JSONException { + final Collection collection = getCollection(dataStoreName); + + final Query query = + Query.builder() + .setFilter( + not( + DocumentArrayFilterExpression.builder() + .operator(ANY) + .arraySource(IdentifierExpression.of("additional_info.planets")) + .filter( + and( + ArrayRelationalFilterExpression.builder() + .operator(ANY) + .filter( + RelationalExpression.of( + IdentifierExpression.of("elements"), + RelationalOperator.EQ, + ConstantExpression.of("Oxygen"))) + .build(), + ArrayRelationalFilterExpression.builder() + .operator(ANY) + .filter( + RelationalExpression.of( + IdentifierExpression.of("elements"), + RelationalOperator.EQ, + ConstantExpression.of("Water"))) + .build(), + RelationalExpression.of( + IdentifierExpression.of("meta.num_moons"), + RelationalOperator.GT, + ConstantExpression.of(0)))) + .build())) + .build(); + + final Iterator documents = collection.aggregate(query); + final String expected = readResource("no_planet_having_both_oxygen_and_water_together.json"); + final String actual = iteratorToJson(documents); + + JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT); + } + private String readResource(final String fileName) { try { return new String( diff --git a/document-store/src/integrationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json b/document-store/src/integrationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json new file mode 100644 index 00000000..4d539d96 --- /dev/null +++ b/document-store/src/integrationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json @@ -0,0 +1,31 @@ +[ + { + "name": "Solar System 1", + "additional_info": { + "planets": [ + { + "name": "Planet 1", + "elements": [ + "Oxygen", + "Water", + "Nitrogen" + ], + "meta": { + "num_moons": 7 + } + }, + { + "name": "Planet 2", + "elements": [ + "Oxygen", + "Helium", + "Water" + ], + "meta": { + "num_moons": 1 + } + } + ] + } + } +] diff --git a/document-store/src/integrationTest/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json b/document-store/src/integrationTest/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json new file mode 100644 index 00000000..2972a3f1 --- /dev/null +++ b/document-store/src/integrationTest/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json @@ -0,0 +1,56 @@ +[ + { + "name": "Solar System 2", + "additional_info": { + "planets": [ + { + "name": "Planet 1" + }, + { + "name": "Planet 2" + }, + { + "name": "Planet 3", + "elements": [ + "Oxygen", + "Nitrogen" + ] + }, + { + "name": "Planet 4", + "elements": [ + "Iron" + ] + }, + { + "name": "Planet 5", + "elements": [] + }, + { + "name": "Planet 6", + "elements": [] + }, + { + "name": "Planet 7", + "elements": [ + "Hydrogen", + "Helium", + "Methane" + ], + "meta": { + "num_moons": 10 + } + }, + { + "name": "Planet 8", + "elements": [ + "Hydrogen", + "Helium", + "Methane", + "Ammonia" + ] + } + ] + } + } +] diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java index 1cc6a085..7baa3b38 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java @@ -3,6 +3,7 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toUnmodifiableList; import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.AND; +import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT; import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.OR; import com.google.common.base.Preconditions; @@ -74,6 +75,10 @@ public static LogicalExpression or( return buildLogicalExpression(operand1, operand2, otherOperands, OR); } + public static LogicalExpression not(final FilterTypeExpression operand) { + return buildLogicalExpression(List.of(operand), NOT); + } + private static LogicalExpression buildLogicalExpression( final FilterTypeExpression operand1, final FilterTypeExpression operand2, @@ -102,6 +107,10 @@ public T accept(final FilterTypeExpressionVisitor visitor) { @Override public String toString() { + if (NOT.equals(operator)) { + return String.format("NOT (%s)", operands.get(0)); + } + return "(" + operands.stream().map(String::valueOf).collect(joining(") " + operator + " (")) + ")"; @@ -109,11 +118,17 @@ public String toString() { public static class LogicalExpressionBuilder { public LogicalExpression build() { + Preconditions.checkArgument(operator != null, "operator is null"); Preconditions.checkArgument(operands != null, "operands is null"); - Preconditions.checkArgument(operands.size() >= 2, "At least 2 operands required"); + + if (NOT.equals(operator)) { + Preconditions.checkArgument(operands.size() == 1, "Exactly one operand required for NOT"); + } else { + Preconditions.checkArgument(operands.size() >= 2, "At least 2 operands required"); + } + Preconditions.checkArgument( operands.stream().noneMatch(Objects::isNull), "One or more operands is null"); - Preconditions.checkArgument(operator != null, "operator is null"); return new LogicalExpression(operands, operator); } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java index b6896cf6..36f91746 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java @@ -3,4 +3,5 @@ public enum LogicalOperator { AND, OR, + NOT, } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java b/document-store/src/main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java index 2803c546..858cbf80 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java @@ -140,7 +140,7 @@ private void applyClusterSettings(final Builder settingsBuilder) { private void applyConnectionPoolSettings(final Builder settingsBuilder) { final ConnectionPoolSettings connectionPoolSettings = ConnectionPoolSettings.builder() - .maxConnecting(connectionPoolConfig.maxConnections()) + .maxSize(connectionPoolConfig.maxConnections()) .maxWaitTime( connectionPoolConfig.connectionAccessTimeout().toMillis(), TimeUnit.MILLISECONDS) .maxConnectionIdleTime( diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java b/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java index ce925710..25f76bbb 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java @@ -2,8 +2,10 @@ import static java.util.Collections.unmodifiableMap; import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.AND; +import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT; import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.OR; import static org.hypertrace.core.documentstore.mongo.MongoUtils.getUnsupportedOperationException; +import static org.hypertrace.core.documentstore.mongo.query.parser.filter.MongoRelationalFilterParserFactory.FilterLocation.INSIDE_EXPR; import java.util.EnumMap; import java.util.List; @@ -15,6 +17,10 @@ import org.hypertrace.core.documentstore.parser.FilterTypeExpressionVisitor; final class MongoLogicalExpressionParser { + + private static final String NOR_OP = "$nor"; + private static final String NOT_OP = "$not"; + private static final Map KEY_MAP = unmodifiableMap( new EnumMap<>(LogicalOperator.class) { @@ -32,7 +38,7 @@ final class MongoLogicalExpressionParser { Map parse(final LogicalExpression expression) { LogicalOperator operator = expression.getOperator(); - String key = KEY_MAP.get(operator); + String key = NOT.equals(operator) ? getNotOp() : KEY_MAP.get(operator); if (key == null) { throw getUnsupportedOperationException(operator); @@ -40,6 +46,7 @@ Map parse(final LogicalExpression expression) { FilterTypeExpressionVisitor parser = new MongoFilterTypeExpressionParser(relationalFilterContext); + List parsed = expression.getOperands().stream() .map(exp -> exp.accept(parser)) @@ -47,4 +54,8 @@ Map parse(final LogicalExpression expression) { return Map.of(key, parsed); } + + private String getNotOp() { + return INSIDE_EXPR.equals(relationalFilterContext.location()) ? NOT_OP : NOR_OP; + } } diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java index bfac00e2..9e017c34 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java @@ -1,6 +1,7 @@ package org.hypertrace.core.documentstore.postgres.query.v1.vistors; import static java.util.stream.Collectors.toUnmodifiableList; +import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT; import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EQ; import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.IN; import static org.hypertrace.core.documentstore.postgres.PostgresCollection.ID; @@ -45,6 +46,10 @@ public PostgresFilterTypeExpressionVisitor( @SuppressWarnings("unchecked") @Override public String visit(final LogicalExpression expression) { + if (NOT.equals(expression.getOperator())) { + return String.format("NOT (%s)", expression.getOperands().get(0).accept(this).toString()); + } + Collector collector = getCollectorForLogicalOperator(expression.getOperator()); String childList = diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java index b2c017ae..cbdbcaf0 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java @@ -27,7 +27,7 @@ void testAllDefaults() { .applyToConnectionPoolSettings( builder -> builder - .maxConnecting(16) + .maxSize(16) .maxWaitTime(10, TimeUnit.SECONDS) .maxConnectionIdleTime(5, TimeUnit.MINUTES)) .build(); @@ -81,7 +81,7 @@ void testAllAssigned() { .applyToConnectionPoolSettings( builder -> builder - .maxConnecting(maxPoolConnections) + .maxSize(maxPoolConnections) .maxWaitTime(maxWaitTime.toSeconds(), TimeUnit.SECONDS) .maxConnectionIdleTime(maxIdleTime.toSeconds(), TimeUnit.SECONDS)) .build(); @@ -124,7 +124,7 @@ void testMissingAuthDb_shouldUseTheSameDatabaseUsedForConnecting() { .applyToConnectionPoolSettings( builder -> builder - .maxConnecting(16) + .maxSize(16) .maxWaitTime(10, TimeUnit.SECONDS) .maxConnectionIdleTime(5, TimeUnit.MINUTES)) .build(); @@ -159,7 +159,7 @@ void testDefaultCredentials_shouldNotSetCredentials() { .applyToConnectionPoolSettings( builder -> builder - .maxConnecting(16) + .maxSize(16) .maxWaitTime(10, TimeUnit.SECONDS) .maxConnectionIdleTime(5, TimeUnit.MINUTES)) .build();