From b5f645bae637db22be2822d8f91d2724b9311767 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Fri, 13 May 2022 12:51:06 -0700 Subject: [PATCH 1/6] Added unit tests for match function implementation and added check to avoid NoSuchElementException in MatchQuery Signed-off-by: forestmvey --- .../filter/lucene/relevance/MatchQuery.java | 3 + .../script/filter/lucene/MatchQueryTest.java | 201 ++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java index bcdcc9f296..e5c6b40eea 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java @@ -63,6 +63,9 @@ public class MatchQuery extends LuceneQuery { @Override public QueryBuilder build(FunctionExpression func) { + if(func.getArguments().size() < 2) { + throw new SemanticCheckException("match must have at least two arguments"); + } Iterator iterator = func.getArguments().iterator(); NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java new file mode 100644 index 0000000000..25812835a3 --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -0,0 +1,201 @@ +package org.opensearch.sql.opensearch.storage.script.filter.lucene; + + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.expression.config.ExpressionConfig; +import org.opensearch.sql.expression.env.Environment; +import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchQuery; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +public class MatchQueryTest { + + private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); + private final MatchQuery matchQuery = new MatchQuery(); + private final FunctionName match = FunctionName.of("match"); + + private NamedArgumentExpression namedArgument(String name, String value) { + return dsl.namedArgument(name, DSL.literal(value)); + } + + @Test + public void test_SemanticCheckException_when_no_arguments() { + List arguments = List.of(); + assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_one_argument() { + List arguments = List.of(namedArgument("field", "field_value")); + assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_invalid_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("unsupported", "unsupported_value")); + Assertions.assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_analyzer_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("analyzer", "standard") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void build_succeeds_with_two_arguments() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value")); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_auto_generate_synonyms_phrase_query_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("auto_generate_synonyms_phrase_query", "true") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzziness_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzziness", "AUTO") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_max_expansions_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("max_expansions", "50") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_prefix_length_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("prefix_length", "0") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzzy_transpositions_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzzy_transpositions", "true") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzzy_rewrite_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzzy_rewrite", "constant_score") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_lenient_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("lenient", "false") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_operator_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("operator", "OR") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_minimum_should_match_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("minimum_should_match", "3") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_zero_terms_query_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("zero_terms_query", "NONE") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_boost_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("boost", "1") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + private class MatchExpression extends FunctionExpression { + public MatchExpression(List arguments) { + super(MatchQueryTest.this.match, arguments); + } + + @Override + public ExprValue valueOf(Environment valueEnv) { + return null; + } + + @Override + public ExprType type() { + return null; + } + } +} From cd518a08e670ba4c6406a19d8062107588c2b1d2 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Fri, 13 May 2022 15:18:50 -0700 Subject: [PATCH 2/6] Add missing copyright to MatchQueryTest Signed-off-by: forestmvey --- .../storage/script/filter/lucene/MatchQueryTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index 25812835a3..fb298e2813 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -1,5 +1,9 @@ -package org.opensearch.sql.opensearch.storage.script.filter.lucene; +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +package org.opensearch.sql.opensearch.storage.script.filter.lucene; import static org.junit.jupiter.api.Assertions.assertThrows; From dc5172ef0ff1d340111d3a2f037d3122a6413198 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Mon, 16 May 2022 09:46:23 -0700 Subject: [PATCH 3/6] Fix build not passing checkstyle syntax Signed-off-by: forestmvey --- .../filter/lucene/relevance/MatchQuery.java | 2 +- .../script/filter/lucene/MatchQueryTest.java | 351 +++++++++--------- 2 files changed, 176 insertions(+), 177 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java index e5c6b40eea..a1ed67cd9b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java @@ -63,7 +63,7 @@ public class MatchQuery extends LuceneQuery { @Override public QueryBuilder build(FunctionExpression func) { - if(func.getArguments().size() < 2) { + if (func.getArguments().size() < 2) { throw new SemanticCheckException("match must have at least two arguments"); } Iterator iterator = func.getArguments().iterator(); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index fb298e2813..d8a01c75d6 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -26,180 +26,179 @@ @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) public class MatchQueryTest { - - private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); - private final MatchQuery matchQuery = new MatchQuery(); - private final FunctionName match = FunctionName.of("match"); - - private NamedArgumentExpression namedArgument(String name, String value) { - return dsl.namedArgument(name, DSL.literal(value)); - } - - @Test - public void test_SemanticCheckException_when_no_arguments() { - List arguments = List.of(); - assertThrows(SemanticCheckException.class, - () -> matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_SemanticCheckException_when_one_argument() { - List arguments = List.of(namedArgument("field", "field_value")); - assertThrows(SemanticCheckException.class, - () -> matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_SemanticCheckException_when_invalid_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("unsupported", "unsupported_value")); - Assertions.assertThrows(SemanticCheckException.class, - () -> matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_analyzer_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("analyzer", "standard") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void build_succeeds_with_two_arguments() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value")); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_auto_generate_synonyms_phrase_query_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("auto_generate_synonyms_phrase_query", "true") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzziness_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzziness", "AUTO") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_max_expansions_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("max_expansions", "50") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_prefix_length_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("prefix_length", "0") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzzy_transpositions_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzzy_transpositions", "true") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzzy_rewrite_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzzy_rewrite", "constant_score") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_lenient_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("lenient", "false") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_operator_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("operator", "OR") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_minimum_should_match_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("minimum_should_match", "3") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_zero_terms_query_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("zero_terms_query", "NONE") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_boost_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("boost", "1") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - private class MatchExpression extends FunctionExpression { - public MatchExpression(List arguments) { - super(MatchQueryTest.this.match, arguments); - } - - @Override - public ExprValue valueOf(Environment valueEnv) { - return null; - } - - @Override - public ExprType type() { - return null; - } - } + private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); + private final MatchQuery matchQuery = new MatchQuery(); + private final FunctionName match = FunctionName.of("match"); + + private NamedArgumentExpression namedArgument(String name, String value) { + return dsl.namedArgument(name, DSL.literal(value)); + } + + @Test + public void test_SemanticCheckException_when_no_arguments() { + List arguments = List.of(); + assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_one_argument() { + List arguments = List.of(namedArgument("field", "field_value")); + assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_invalid_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("unsupported", "unsupported_value")); + Assertions.assertThrows(SemanticCheckException.class, + () -> matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_analyzer_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("analyzer", "standard") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void build_succeeds_with_two_arguments() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value")); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_auto_generate_synonyms_phrase_query_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("auto_generate_synonyms_phrase_query", "true") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzziness_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzziness", "AUTO") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_max_expansions_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("max_expansions", "50") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_prefix_length_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("prefix_length", "0") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzzy_transpositions_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzzy_transpositions", "true") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_fuzzy_rewrite_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("fuzzy_rewrite", "constant_score") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_lenient_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("lenient", "false") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_operator_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("operator", "OR") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_minimum_should_match_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("minimum_should_match", "3") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_zero_terms_query_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("zero_terms_query", "NONE") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + @Test + public void test_boost_parameter() { + List arguments = List.of( + namedArgument("field", "field_value"), + namedArgument("query", "query_value"), + namedArgument("boost", "1") + ); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } + + private class MatchExpression extends FunctionExpression { + public MatchExpression(List arguments) { + super(MatchQueryTest.this.match, arguments); + } + + @Override + public ExprValue valueOf(Environment valueEnv) { + return null; + } + + @Override + public ExprType type() { + return null; + } + } } From c54a11f55591bd3a1ae010960f269991a2dc390f Mon Sep 17 00:00:00 2001 From: forestmvey Date: Mon, 16 May 2022 13:49:21 -0700 Subject: [PATCH 4/6] Made invalid function implementations intentions more clear by throwing exceptions and moved private method definition placement below public methods Signed-off-by: forestmvey --- .../storage/script/filter/lucene/MatchQueryTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index d8a01c75d6..75ca714fdd 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -30,10 +30,6 @@ public class MatchQueryTest { private final MatchQuery matchQuery = new MatchQuery(); private final FunctionName match = FunctionName.of("match"); - private NamedArgumentExpression namedArgument(String name, String value) { - return dsl.namedArgument(name, DSL.literal(value)); - } - @Test public void test_SemanticCheckException_when_no_arguments() { List arguments = List.of(); @@ -186,6 +182,10 @@ public void test_boost_parameter() { Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); } + private NamedArgumentExpression namedArgument(String name, String value) { + return dsl.namedArgument(name, DSL.literal(value)); + } + private class MatchExpression extends FunctionExpression { public MatchExpression(List arguments) { super(MatchQueryTest.this.match, arguments); @@ -193,12 +193,12 @@ public MatchExpression(List arguments) { @Override public ExprValue valueOf(Environment valueEnv) { - return null; + throw new UnsupportedOperationException("Invalid function call, valueOf function need implementation only to support Expression interface"); } @Override public ExprType type() { - return null; + throw new UnsupportedOperationException("Invalid function call, type function need implementation only to support Expression interface"); } } } From 34d395c2e0ae13f84c58ad18a9abad16a109a2f0 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Tue, 17 May 2022 10:33:39 -0700 Subject: [PATCH 5/6] Replaced modular tests for valid arguments with iterative substitute. Fixed checkstyle syntax errors. Signed-off-by: forestmvey --- .../script/filter/lucene/MatchQueryTest.java | 149 ++++-------------- 1 file changed, 30 insertions(+), 119 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index 75ca714fdd..ecbc913517 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayNameGeneration; @@ -54,16 +55,6 @@ public void test_SemanticCheckException_when_invalid_parameter() { () -> matchQuery.build(new MatchExpression(arguments))); } - @Test - public void test_analyzer_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("analyzer", "standard") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - @Test public void build_succeeds_with_two_arguments() { List arguments = List.of( @@ -73,113 +64,31 @@ public void build_succeeds_with_two_arguments() { } @Test - public void test_auto_generate_synonyms_phrase_query_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("auto_generate_synonyms_phrase_query", "true") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzziness_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzziness", "AUTO") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_max_expansions_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("max_expansions", "50") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_prefix_length_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("prefix_length", "0") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzzy_transpositions_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzzy_transpositions", "true") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_fuzzy_rewrite_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("fuzzy_rewrite", "constant_score") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_lenient_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("lenient", "false") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_operator_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("operator", "OR") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_minimum_should_match_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("minimum_should_match", "3") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_zero_terms_query_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("zero_terms_query", "NONE") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_boost_parameter() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value"), - namedArgument("boost", "1") - ); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + public void test_valid_parameters() { + final int validArgIndex = 2; + final List validArgs = List.of( + namedArgument("analyzer", "standard"), + namedArgument("auto_generate_synonyms_phrase_query", "true"), + namedArgument("fuzziness", "AUTO"), + namedArgument("max_expansions", "50"), + namedArgument("prefix_length", "0"), + namedArgument("fuzzy_transpositions", "true"), + namedArgument("fuzzy_rewrite", "constant_score"), + namedArgument("lenient", "false"), + namedArgument("operator", "OR"), + namedArgument("minimum_should_match", "3"), + namedArgument("zero_terms_query", "NONE"), + namedArgument("boost", "1")); + + List arguments = new ArrayList(); + arguments.add(namedArgument("field", "field_value")); + arguments.add(namedArgument("query", "query_value")); + arguments.add(namedArgument("", "")); + + for (Expression arg : validArgs) { + arguments.set(validArgIndex, arg); + Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); + } } private NamedArgumentExpression namedArgument(String name, String value) { @@ -193,12 +102,14 @@ public MatchExpression(List arguments) { @Override public ExprValue valueOf(Environment valueEnv) { - throw new UnsupportedOperationException("Invalid function call, valueOf function need implementation only to support Expression interface"); + throw new UnsupportedOperationException("Invalid function call, " + + "valueOf function need implementation only to support Expression interface"); } @Override public ExprType type() { - throw new UnsupportedOperationException("Invalid function call, type function need implementation only to support Expression interface"); + throw new UnsupportedOperationException("Invalid function call, " + + "type function need implementation only to support Expression interface"); } } } From 4fa257817648ab050c291f886c39eeb77d8f9260 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Wed, 18 May 2022 11:23:29 -0700 Subject: [PATCH 6/6] Reworking valid tests to parameterized tests Signed-off-by: forestmvey --- .../script/filter/lucene/MatchQueryTest.java | 116 ++++++++++++------ 1 file changed, 79 insertions(+), 37 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index ecbc913517..7e3e5e0862 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -7,12 +7,14 @@ import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.ArrayList; import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.SemanticCheckException; @@ -31,6 +33,82 @@ public class MatchQueryTest { private final MatchQuery matchQuery = new MatchQuery(); private final FunctionName match = FunctionName.of("match"); + static Stream> generateValidData() { + final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); + return Stream.of( + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("analyzer", DSL.literal("standard")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("auto_generate_synonyms_phrase_query", DSL.literal("true")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("fuzziness", DSL.literal("AUTO")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("max_expansions", DSL.literal("50")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("prefix_length", DSL.literal("0")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("fuzzy_transpositions", DSL.literal("true")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("fuzzy_rewrite", DSL.literal("constant_score")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("lenient", DSL.literal("false")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("operator", DSL.literal("OR")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("minimum_should_match", DSL.literal("3")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("zero_terms_query", DSL.literal("NONE")) + ), + List.of( + dsl.namedArgument("field", DSL.literal("field_value")), + dsl.namedArgument("query", DSL.literal("query_value")), + dsl.namedArgument("boost", DSL.literal("1")) + ) + ); + } + + @ParameterizedTest + @MethodSource("generateValidData") + public void test_valid_parameters(List validArgs) { + Assertions.assertNotNull(matchQuery.build(new MatchExpression(validArgs))); + } + @Test public void test_SemanticCheckException_when_no_arguments() { List arguments = List.of(); @@ -55,42 +133,6 @@ public void test_SemanticCheckException_when_invalid_parameter() { () -> matchQuery.build(new MatchExpression(arguments))); } - @Test - public void build_succeeds_with_two_arguments() { - List arguments = List.of( - namedArgument("field", "field_value"), - namedArgument("query", "query_value")); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - - @Test - public void test_valid_parameters() { - final int validArgIndex = 2; - final List validArgs = List.of( - namedArgument("analyzer", "standard"), - namedArgument("auto_generate_synonyms_phrase_query", "true"), - namedArgument("fuzziness", "AUTO"), - namedArgument("max_expansions", "50"), - namedArgument("prefix_length", "0"), - namedArgument("fuzzy_transpositions", "true"), - namedArgument("fuzzy_rewrite", "constant_score"), - namedArgument("lenient", "false"), - namedArgument("operator", "OR"), - namedArgument("minimum_should_match", "3"), - namedArgument("zero_terms_query", "NONE"), - namedArgument("boost", "1")); - - List arguments = new ArrayList(); - arguments.add(namedArgument("field", "field_value")); - arguments.add(namedArgument("query", "query_value")); - arguments.add(namedArgument("", "")); - - for (Expression arg : validArgs) { - arguments.set(validArgIndex, arg); - Assertions.assertNotNull(matchQuery.build(new MatchExpression(arguments))); - } - } - private NamedArgumentExpression namedArgument(String name, String value) { return dsl.namedArgument(name, DSL.literal(value)); }