From 61164e7c1bf975c62af23af8106ebb9268bb2a24 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Fri, 27 Jan 2023 21:02:33 -0500 Subject: [PATCH] Changes to support $eq comparison operator. (#55) --- .../sgv3/docsapi/StargateDocsV3Api.java | 4 +- .../clause/filter/ComparisonExpression.java | 15 +++++- .../filter/ValueComparisonOperator.java | 45 +++++++++++++--- .../FilterClauseDeserializer.java | 41 +++++++++++++-- .../model/impl/matcher/FilterMatchRules.java | 6 ++- .../filter/ComparisonExpressionTest.java | 19 +++++-- .../FilterClauseDeserializerTest.java | 17 +++++++ .../docsapi/api/v3/FindIntegrationTest.java | 51 +++++++++++++++++++ 8 files changed, 179 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/stargate/sgv3/docsapi/StargateDocsV3Api.java b/src/main/java/io/stargate/sgv3/docsapi/StargateDocsV3Api.java index cf0257341f..d24aa9fa17 100644 --- a/src/main/java/io/stargate/sgv3/docsapi/StargateDocsV3Api.java +++ b/src/main/java/io/stargate/sgv3/docsapi/StargateDocsV3Api.java @@ -58,7 +58,7 @@ """ { "findOne": { - "filter": {"location": "London"} + "filter": {"location": "London", "race.competitors" : {"$eq" : 100}} } } """), @@ -69,7 +69,7 @@ """ { "find": { - "filter": {"location": "London"}, + "filter": {"location": "London", "race.competitors" : {"$eq" : 100}}, "options": {"limit" : 1000, "pageSize": 25, "pagingState" : "Next paging state got from previous page call"} } } diff --git a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpression.java b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpression.java index 812ecca59b..43ffe676e2 100644 --- a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpression.java +++ b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpression.java @@ -21,7 +21,7 @@ public record ComparisonExpression( @Valid @NotEmpty List> filterOperations) { /** - * Shortcut to create equals against a literal + * Shortcut to create equals against a literal, mare condition cannot be added using add(). * *

e.g. {"username" : "aaron"} * @@ -35,6 +35,19 @@ public static ComparisonExpression eq(String path, Object value) { List.of(new ValueComparisonOperation<>(ValueComparisonOperator.EQ, getLiteral(value)))); } + /** + * Adds a comparison operation + * + *

e.g. {"username" : "aaron"} + * + * @param path Json node path + * @param value Value returned by the deserializer + * @return {@link ComparisonExpression} with equal operator + */ + public void add(ValueComparisonOperator operator, Object value) { + filterOperations.add(new ValueComparisonOperation<>(operator, getLiteral(value))); + } + /** * Create Typed JsonLiteral object for the value * diff --git a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ValueComparisonOperator.java b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ValueComparisonOperator.java index 3e31846ed3..4e2071db3b 100644 --- a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ValueComparisonOperator.java +++ b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ValueComparisonOperator.java @@ -1,11 +1,42 @@ package io.stargate.sgv3.docsapi.api.model.command.clause.filter; -/** List of value operator that can be used in Filter clause */ +import io.stargate.sgv3.docsapi.exception.DocsException; +import io.stargate.sgv3.docsapi.exception.ErrorCode; +import java.util.HashMap; +import java.util.Map; + +/** + * List of value operator that can be used in Filter clause Have commented the unsupported + * operators, will add it as we support them + */ public enum ValueComparisonOperator implements FilterOperator { - EQ, - GT, - GTE, - LT, - LTE, - NE + EQ("$eq"); + /*GT("$gt"), + GTE("$gte"), + LT("$lt"), + LTE("$lte"), + NE("$ne");*/ + + private String operator; + + ValueComparisonOperator(String operator) { + this.operator = operator; + } + + private static final Map operatorMap = new HashMap<>(); + + static { + for (ValueComparisonOperator filterOperator : ValueComparisonOperator.values()) { + operatorMap.put(filterOperator.operator, filterOperator); + } + } + + public static ValueComparisonOperator getComparisonOperator(String operator) { + final ValueComparisonOperator valueComparisonOperator = operatorMap.get(operator); + if (valueComparisonOperator == null) + throw new DocsException( + ErrorCode.UNSUPPORTED_FILTER_OPERATION, "Unsupported filter operation " + operator); + + return valueComparisonOperator; + } } diff --git a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializer.java index 52f42ff505..8904fd0509 100644 --- a/src/main/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.databind.deser.std.StdDeserializer; import io.stargate.sgv3.docsapi.api.model.command.clause.filter.ComparisonExpression; import io.stargate.sgv3.docsapi.api.model.command.clause.filter.FilterClause; +import io.stargate.sgv3.docsapi.api.model.command.clause.filter.ValueComparisonOperator; import io.stargate.sgv3.docsapi.exception.DocsException; import io.stargate.sgv3.docsapi.exception.ErrorCode; import java.io.IOException; @@ -22,7 +23,10 @@ public FilterClauseDeserializer() { super(FilterClause.class); } - /** {@inheritDoc} */ + /** + * {@inheritDoc} Filter clause can follow short cut {"field" : "value"} instead of {"field" : + * {"$eq" : "value"}} + */ @Override public FilterClause deserialize( JsonParser jsonParser, DeserializationContext deserializationContext) @@ -35,13 +39,42 @@ public FilterClause deserialize( Map.Entry entry = fieldIter.next(); // TODO: Does not handle logical expressions, they are out of scope JsonNode operatorExpression = entry.getValue(); - if (!operatorExpression.isValueNode()) - throw new DocsException(ErrorCode.UNSUPPORTED_FILTER_DATA_TYPE); - expressionList.add(ComparisonExpression.eq(entry.getKey(), jsonNodeValue(entry.getValue()))); + if (operatorExpression.isObject()) { + expressionList.add(createComparisonExpression(entry)); + } else { + // @TODO: Need to add array value type to this condition + expressionList.add( + ComparisonExpression.eq(entry.getKey(), jsonNodeValue(entry.getValue()))); + } } return new FilterClause(expressionList); } + /** + * The filter clause is entry will have field path as key and object type as value. The value can + * have multiple operator and condition values. + * + *

Eg 1: {"field" : {"$eq" : "value"}} + * + *

Eg 2: {"field" : {"$gt" : 10, "$lt" : 50}} + * + * @param entry + * @return + */ + private ComparisonExpression createComparisonExpression(Map.Entry entry) { + ComparisonExpression expression = new ComparisonExpression(entry.getKey(), new ArrayList<>()); + final Iterator> fields = entry.getValue().fields(); + while (fields.hasNext()) { + Map.Entry updateField = fields.next(); + ValueComparisonOperator operator = + ValueComparisonOperator.getComparisonOperator(updateField.getKey()); + JsonNode value = updateField.getValue(); + // @TODO: Need to add array and sub-document value type to this condition + expression.add(operator, jsonNodeValue(value)); + } + return expression; + } + private static Object jsonNodeValue(JsonNode node) { switch (node.getNodeType()) { case BOOLEAN: diff --git a/src/main/java/io/stargate/sgv3/docsapi/service/resolver/model/impl/matcher/FilterMatchRules.java b/src/main/java/io/stargate/sgv3/docsapi/service/resolver/model/impl/matcher/FilterMatchRules.java index 8e2a85f148..a426a5d895 100644 --- a/src/main/java/io/stargate/sgv3/docsapi/service/resolver/model/impl/matcher/FilterMatchRules.java +++ b/src/main/java/io/stargate/sgv3/docsapi/service/resolver/model/impl/matcher/FilterMatchRules.java @@ -64,7 +64,11 @@ public ReadOperation apply(CommandContext commandContext, T command) { .filter(Optional::isPresent) .map(Optional::get) // unwraps the Optional from the resolver function. .findFirst() - .orElseThrow(() -> new DocsException(ErrorCode.FILTER_UNRESOLVABLE)); + .orElseThrow( + () -> + new DocsException( + ErrorCode.FILTER_UNRESOLVABLE, + "Filter type not supported, unable to resolve to a filtering strategy")); } @VisibleForTesting diff --git a/src/test/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpressionTest.java b/src/test/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpressionTest.java index 4857da2518..9fcfc9fb9b 100644 --- a/src/test/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpressionTest.java +++ b/src/test/java/io/stargate/sgv3/docsapi/api/model/command/clause/filter/ComparisonExpressionTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.math.BigDecimal; +import java.util.ArrayList; import java.util.EnumSet; import java.util.List; import org.junit.jupiter.api.Nested; @@ -26,6 +27,20 @@ public void stringValueComparisonExpression() throws Exception { assertThat(result).isEqualTo(expectedResult); } + @Test + public void multiValueComparisonExpression() throws Exception { + final ComparisonExpression expectedResult = + new ComparisonExpression( + "username", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("abc", JsonType.STRING)))); + + ComparisonExpression result = new ComparisonExpression("username", new ArrayList<>()); + result.add(ValueComparisonOperator.EQ, "abc"); + assertThat(result).isEqualTo(expectedResult); + } + @Test public void numberValueComparisonExpression() throws Exception { final ComparisonExpression expectedResult = @@ -95,10 +110,6 @@ public void matchTest() throws Exception { comparisonExpression.match( "path", EnumSet.of(ValueComparisonOperator.EQ), JsonType.STRING); assertThat(match).hasSize(0); - - match = - comparisonExpression.match("path", EnumSet.of(ValueComparisonOperator.GT), JsonType.NULL); - assertThat(match).hasSize(0); } } } diff --git a/src/test/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializerTest.java b/src/test/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializerTest.java index feefe27c93..459aa984e6 100644 --- a/src/test/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv3/docsapi/api/model/command/deserializers/FilterClauseDeserializerTest.java @@ -46,6 +46,23 @@ public void happyPath() throws Exception { assertThat(filterClause.comparisonExpressions()).hasSize(1).contains(expectedResult); } + @Test + public void eqComparisonOperator() throws Exception { + String json = """ + {"username": {"$eq" : "aaron"}} + """; + + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + final ComparisonExpression expectedResult = + new ComparisonExpression( + "username", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("aaron", JsonType.STRING)))); + assertThat(filterClause).isNotNull(); + assertThat(filterClause.comparisonExpressions()).hasSize(1).contains(expectedResult); + } + @Test public void mustHandleNull() throws Exception { String json = "null"; diff --git a/src/test/java/io/stargate/sgv3/docsapi/api/v3/FindIntegrationTest.java b/src/test/java/io/stargate/sgv3/docsapi/api/v3/FindIntegrationTest.java index a17e78ccdd..5763874677 100644 --- a/src/test/java/io/stargate/sgv3/docsapi/api/v3/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv3/docsapi/api/v3/FindIntegrationTest.java @@ -5,6 +5,7 @@ import static net.javacrumbs.jsonunit.JsonMatchers.jsonEquals; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.startsWith; import io.quarkus.test.junit.QuarkusIntegrationTest; import io.restassured.http.ContentType; @@ -137,6 +138,7 @@ public void findById() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) + .body("data.count", is(1)) .body("data.docs[0]", jsonEquals(expected)); } @@ -160,9 +162,56 @@ public void findByColumn() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) + .body("data.count", is(1)) .body("data.docs[0]", jsonEquals(expected)); } + @Test + @Order(2) + public void findWithEqComparisonOperator() { + String json = + """ + { + "find": { + "filter" : {"username" : {"$eq" : "user1"}} + } + } + """; + String expected = "{\"_id\":\"doc1\", \"username\":\"user1\", \"active_user\":true}"; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.count", is(1)) + .body("data.docs[0]", jsonEquals(expected)); + } + + @Test + @Order(2) + public void findWithNEComparisonOperator() { + String json = + """ + { + "find": { + "filter" : {"username" : {"$ne" : "user1"}} + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("errors[1].message", startsWith("Unsupported filter operation $ne")); + } + @Test @Order(2) public void findByBooleanColumn() { @@ -280,6 +329,7 @@ public void findOneById() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) + .body("data.count", is(1)) .body("data.docs[0]", jsonEquals(expected)); } @@ -303,6 +353,7 @@ public void findOneByColumn() { .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) .then() .statusCode(200) + .body("data.count", is(1)) .body("data.docs[0]", jsonEquals(expected)); } }