diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c08c34a1a3c..2f38de8c113b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Search Stats] Add search & star-tree search query failure count metrics ([#19210](https://github.com/opensearch-project/OpenSearch/issues/19210)) - [Star-tree] Support for multi-terms aggregation ([#18398](https://github.com/opensearch-project/OpenSearch/issues/18398)) - Add stream search feature flag and auto fallback logic ([#19373](https://github.com/opensearch-project/OpenSearch/pull/19373)) +- Implement GRPC Exists, Regexp, and Wildcard queries ([#19392](https://github.com/opensearch-project/OpenSearch/pull/19392)) +- Implement GRPC GeoBoundingBox, GeoDistance queries ([#19451](https://github.com/opensearch-project/OpenSearch/pull/19451)) +- Implement GRPC Ids, Range, and Terms Set queries ([#19448](https://github.com/opensearch-project/OpenSearch/pull/19448)) +- Implement GRPC Nested query ([#19453](https://github.com/opensearch-project/OpenSearch/pull/19453)) ### Changed - Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965)) @@ -59,6 +63,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Migrate usages of deprecated `Operations#union` from Lucene ([#19397](https://github.com/opensearch-project/OpenSearch/pull/19397)) - Delegate primitive write methods with ByteSizeCachingDirectory wrapped IndexOutput ([#19432](https://github.com/opensearch-project/OpenSearch/pull/19432)) - Bump opensearch-protobufs dependency to 0.18.0 and update transport-grpc module compatibility ([#19447](https://github.com/opensearch-project/OpenSearch/issues/19447)) +- Bump opensearch-protobufs dependency to 0.19.0 ([#19453](https://github.com/opensearch-project/OpenSearch/issues/19453)) ### Fixed - Fix unnecessary refreshes on update preparation failures ([#15261](https://github.com/opensearch-project/OpenSearch/issues/15261)) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c0cc98edc50f0..5314be5d3c327 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -22,7 +22,7 @@ kotlin = "1.7.10" antlr4 = "4.13.1" guava = "33.2.1-jre" gson = "2.13.2" -opensearchprotobufs = "0.18.0" +opensearchprotobufs = "0.19.0" protobuf = "3.25.8" jakarta_annotation = "1.3.5" google_http_client = "1.44.1" diff --git a/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 b/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 deleted file mode 100644 index 8b343afc9f899..0000000000000 --- a/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -e8dc93c60df892184d7c2010e1bd4f15a777c292 \ No newline at end of file diff --git a/modules/transport-grpc/licenses/protobufs-0.19.0.jar.sha1 b/modules/transport-grpc/licenses/protobufs-0.19.0.jar.sha1 new file mode 100644 index 0000000000000..c6a8591323db5 --- /dev/null +++ b/modules/transport-grpc/licenses/protobufs-0.19.0.jar.sha1 @@ -0,0 +1 @@ +14b425a0cb280ccad20983daeaabf3d1c83c3670 \ No newline at end of file diff --git a/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 b/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 deleted file mode 100644 index 8b343afc9f899..0000000000000 --- a/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -e8dc93c60df892184d7c2010e1bd4f15a777c292 \ No newline at end of file diff --git a/modules/transport-grpc/spi/licenses/protobufs-0.19.0.jar.sha1 b/modules/transport-grpc/spi/licenses/protobufs-0.19.0.jar.sha1 new file mode 100644 index 0000000000000..c6a8591323db5 --- /dev/null +++ b/modules/transport-grpc/spi/licenses/protobufs-0.19.0.jar.sha1 @@ -0,0 +1 @@ +14b425a0cb280ccad20983daeaabf3d1c83c3670 \ No newline at end of file diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java index 344f33d6a0373..bd55a199b40ab 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java @@ -13,6 +13,7 @@ import org.opensearch.search.collapse.CollapseBuilder; import java.io.IOException; +import java.util.ArrayList; import java.util.List; /** @@ -45,7 +46,10 @@ protected static CollapseBuilder fromProto(FieldCollapse collapseProto) throws I collapseBuilder.setMaxConcurrentGroupRequests(collapseProto.getMaxConcurrentGroupSearches()); } if (collapseProto.getInnerHitsCount() > 0) { - List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(collapseProto.getInnerHitsList()); + List innerHitBuilders = new ArrayList<>(); + for (org.opensearch.protobufs.InnerHits innerHits : collapseProto.getInnerHitsList()) { + innerHitBuilders.add(InnerHitsBuilderProtoUtils.fromProto(innerHits)); + } collapseBuilder.setInnerHits(innerHitBuilders); } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtils.java index b90102b209c6b..a5deb65200966 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtils.java @@ -30,10 +30,18 @@ private FieldAndFormatProtoUtils() { * @param fieldAndFormatProto The Protocol Buffer FieldAndFormat to convert * @return A configured FieldAndFormat instance */ - protected static FieldAndFormat fromProto(org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto) { + public static FieldAndFormat fromProto(org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto) { + if (fieldAndFormatProto == null) { + throw new IllegalArgumentException("FieldAndFormat protobuf cannot be null"); + } - // TODO how is this field used? - // fieldAndFormatProto.getIncludeUnmapped(); - return new FieldAndFormat(fieldAndFormatProto.getField(), fieldAndFormatProto.getFormat()); + String fieldName = fieldAndFormatProto.getField(); + if (fieldName == null || fieldName.trim().isEmpty()) { + throw new IllegalArgumentException("Field name cannot be null or empty"); + } + String format = fieldAndFormatProto.hasFormat() ? fieldAndFormatProto.getFormat() : null; + + return new FieldAndFormat(fieldName, format); } + } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java index d6a0ad09fb5c7..d846700cd9096 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java @@ -75,19 +75,18 @@ public static InnerHitBuilder fromProto(InnerHits innerHits) throws IOException innerHitBuilder.setStoredFieldNames(innerHits.getStoredFieldsList()); } if (innerHits.getDocvalueFieldsCount() > 0) { - List fieldAndFormatList = new ArrayList<>(); + List docvalueFieldsList = new ArrayList<>(); for (org.opensearch.protobufs.FieldAndFormat fieldAndFormat : innerHits.getDocvalueFieldsList()) { - fieldAndFormatList.add(FieldAndFormatProtoUtils.fromProto(fieldAndFormat)); + docvalueFieldsList.add(FieldAndFormatProtoUtils.fromProto(fieldAndFormat)); } - innerHitBuilder.setDocValueFields(fieldAndFormatList); + innerHitBuilder.setDocValueFields(docvalueFieldsList); } if (innerHits.getFieldsCount() > 0) { - List fieldAndFormatList = new ArrayList<>(); - // TODO: this is not correct, we need to use FieldAndFormatProtoUtils.fromProto() and fix the protobufs in 0.11.0 - for (String fieldName : innerHits.getFieldsList()) { - fieldAndFormatList.add(new FieldAndFormat(fieldName, null)); + List fieldsList = new ArrayList<>(); + for (org.opensearch.protobufs.FieldAndFormat fieldAndFormat : innerHits.getFieldsList()) { + fieldsList.add(FieldAndFormatProtoUtils.fromProto(fieldAndFormat)); } - innerHitBuilder.setFetchFields(fieldAndFormatList); + innerHitBuilder.setFetchFields(fieldsList); } if (innerHits.getScriptFieldsCount() > 0) { Set scriptFields = new HashSet<>(); @@ -118,24 +117,4 @@ public static InnerHitBuilder fromProto(InnerHits innerHits) throws IOException return innerHitBuilder; } - /** - * Converts a list of protobuf InnerHits to a list of OpenSearch InnerHitBuilder objects. - * Each InnerHits protobuf message represents ONE inner hit definition. - * - * @param innerHitsList the list of protobuf InnerHits to convert - * @return the list of converted OpenSearch InnerHitBuilder objects - * @throws IOException if there's an error during parsing - */ - public static List fromProto(List innerHitsList) throws IOException { - if (innerHitsList == null) { - throw new IllegalArgumentException("InnerHits list cannot be null"); - } - - List innerHitBuilders = new ArrayList<>(); - for (InnerHits innerHits : innerHitsList) { - innerHitBuilders.add(fromProto(innerHits)); - } - return innerHitBuilders; - } - } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverter.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..3defd30ef9fd5 --- /dev/null +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverter.java @@ -0,0 +1,47 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverter; +import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry; + +/** + * Converter for NestedQuery protobuf messages to OpenSearch QueryBuilder objects. + * Handles the conversion of nested query protobuf messages to OpenSearch NestedQueryBuilder. + */ +public class NestedQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + /** + * Default constructor for NestedQueryBuilderProtoConverter. + */ + public NestedQueryBuilderProtoConverter() {} + + private QueryBuilderProtoConverterRegistry registry; + + @Override + public void setRegistry(QueryBuilderProtoConverterRegistry registry) { + this.registry = registry; + // The utility class no longer stores the registry statically, it's passed directly to fromProto + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.NESTED; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || queryContainer.getQueryContainerCase() != QueryContainer.QueryContainerCase.NESTED) { + throw new IllegalArgumentException("QueryContainer must contain a NestedQuery"); + } + return NestedQueryBuilderProtoUtils.fromProto(queryContainer.getNested(), registry); + } +} diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtils.java new file mode 100644 index 0000000000000..f3e5fe00be564 --- /dev/null +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtils.java @@ -0,0 +1,123 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc.proto.request.search.query; + +import org.apache.lucene.search.join.ScoreMode; +import org.opensearch.index.query.AbstractQueryBuilder; +import org.opensearch.index.query.InnerHitBuilder; +import org.opensearch.index.query.NestedQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.ChildScoreMode; +import org.opensearch.protobufs.NestedQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.transport.grpc.proto.request.search.InnerHitsBuilderProtoUtils; +import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry; + +/** + * Utility class for converting protobuf NestedQuery to OpenSearch NestedQueryBuilder. + * Handles the conversion of nested query protobuf messages to OpenSearch NestedQueryBuilder objects. + */ +class NestedQueryBuilderProtoUtils { + + private NestedQueryBuilderProtoUtils() { + // Utility class, no instances + } + + /** + * Converts a protobuf NestedQuery to an OpenSearch NestedQueryBuilder. + * Similar to {@link NestedQueryBuilder#fromXContent(org.opensearch.core.xcontent.XContentParser)}, this method + * parses the Protocol Buffer representation and creates a properly configured + * NestedQueryBuilder with the appropriate path, query, score mode, inner hits, boost, and query name. + * + * @param nestedQueryProto the protobuf NestedQuery to convert + * @param registry The registry to use for converting nested queries + * @return the converted OpenSearch NestedQueryBuilder + * @throws IllegalArgumentException if the protobuf query is invalid + */ + static NestedQueryBuilder fromProto(NestedQuery nestedQueryProto, QueryBuilderProtoConverterRegistry registry) { + if (nestedQueryProto == null) { + throw new IllegalArgumentException("NestedQuery cannot be null"); + } + + float boost = AbstractQueryBuilder.DEFAULT_BOOST; + ScoreMode scoreMode = ScoreMode.Avg; + String queryName = null; + QueryBuilder query = null; + InnerHitBuilder innerHitBuilder = null; + boolean ignoreUnmapped = NestedQueryBuilder.DEFAULT_IGNORE_UNMAPPED; + + String path = nestedQueryProto.getPath(); + if (path.isEmpty()) { + throw new IllegalArgumentException("Path is required for NestedQuery"); + } + + if (!nestedQueryProto.hasQuery()) { + throw new IllegalArgumentException("Query is required for NestedQuery"); + } + try { + QueryContainer queryContainer = nestedQueryProto.getQuery(); + query = registry.fromProto(queryContainer); + } catch (Exception e) { + throw new IllegalArgumentException("Failed to convert inner query for NestedQuery: " + e.getMessage(), e); + } + + if (nestedQueryProto.hasScoreMode()) { + scoreMode = parseScoreMode(nestedQueryProto.getScoreMode()); + } + + if (nestedQueryProto.hasIgnoreUnmapped()) { + ignoreUnmapped = nestedQueryProto.getIgnoreUnmapped(); + } + + if (nestedQueryProto.hasBoost()) { + boost = nestedQueryProto.getBoost(); + } + + if (nestedQueryProto.hasXName()) { + queryName = nestedQueryProto.getXName(); + } + + if (nestedQueryProto.hasInnerHits()) { + try { + innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(nestedQueryProto.getInnerHits()); + } catch (Exception e) { + throw new IllegalArgumentException("Failed to convert inner hits for NestedQuery: " + e.getMessage(), e); + } + } + + NestedQueryBuilder queryBuilder = new NestedQueryBuilder(path, query, scoreMode, innerHitBuilder).ignoreUnmapped(ignoreUnmapped) + .queryName(queryName) + .boost(boost); + + return queryBuilder; + } + + /** + * Converts protobuf ChildScoreMode to Lucene ScoreMode. + * + * @param childScoreMode the protobuf ChildScoreMode + * @return the converted Lucene ScoreMode + */ + private static ScoreMode parseScoreMode(ChildScoreMode childScoreMode) { + switch (childScoreMode) { + case CHILD_SCORE_MODE_AVG: + return ScoreMode.Avg; + case CHILD_SCORE_MODE_MAX: + return ScoreMode.Max; + case CHILD_SCORE_MODE_MIN: + return ScoreMode.Min; + case CHILD_SCORE_MODE_NONE: + return ScoreMode.None; + case CHILD_SCORE_MODE_SUM: + return ScoreMode.Total; + default: + return ScoreMode.Avg; // Default value + } + } +} diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.java index 905a1a7fce183..cb3270f4035c6 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.java @@ -57,6 +57,7 @@ protected void registerBuiltInConverters() { delegate.registerConverter(new WildcardQueryBuilderProtoConverter()); delegate.registerConverter(new GeoBoundingBoxQueryBuilderProtoConverter()); delegate.registerConverter(new GeoDistanceQueryBuilderProtoConverter()); + delegate.registerConverter(new NestedQueryBuilderProtoConverter()); // Set the registry on all converters so they can access each other delegate.setRegistryOnAllConverters(this); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtilsTests.java new file mode 100644 index 0000000000000..d2f82cf006277 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/FieldAndFormatProtoUtilsTests.java @@ -0,0 +1,79 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc.proto.request.search; + +import org.opensearch.search.fetch.subphase.FieldAndFormat; +import org.opensearch.test.OpenSearchTestCase; + +/** + * Unit tests for FieldAndFormatProtoUtils. + */ +public class FieldAndFormatProtoUtilsTests extends OpenSearchTestCase { + + public void testFromProto_WithFieldOnly() { + org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto = org.opensearch.protobufs.FieldAndFormat.newBuilder() + .setField("test_field") + .build(); + + FieldAndFormat result = FieldAndFormatProtoUtils.fromProto(fieldAndFormatProto); + + assertNotNull(result); + assertEquals("test_field", result.field); + assertNull(result.format); + } + + public void testFromProto_WithFieldAndFormat() { + org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto = org.opensearch.protobufs.FieldAndFormat.newBuilder() + .setField("date_field") + .setFormat("yyyy-MM-dd") + .build(); + + FieldAndFormat result = FieldAndFormatProtoUtils.fromProto(fieldAndFormatProto); + + assertNotNull(result); + assertEquals("date_field", result.field); + assertEquals("yyyy-MM-dd", result.format); + } + + public void testFromProto_WithEmptyFieldName() { + org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto = org.opensearch.protobufs.FieldAndFormat.newBuilder() + .setField("") + .setFormat("yyyy-MM-dd") + .build(); + + try { + FieldAndFormatProtoUtils.fromProto(fieldAndFormatProto); + fail("Should have thrown IllegalArgumentException for empty field name"); + } catch (IllegalArgumentException e) { + assertEquals("Field name cannot be null or empty", e.getMessage()); + } + } + + public void testFromProto_WithNullProtobuf() { + try { + FieldAndFormatProtoUtils.fromProto(null); + fail("Should have thrown IllegalArgumentException for null protobuf"); + } catch (IllegalArgumentException e) { + assertEquals("FieldAndFormat protobuf cannot be null", e.getMessage()); + } + } + + public void testFromProto_WithComplexFormat() { + org.opensearch.protobufs.FieldAndFormat fieldAndFormatProto = org.opensearch.protobufs.FieldAndFormat.newBuilder() + .setField("timestamp") + .setFormat("epoch_millis") + .build(); + + FieldAndFormat result = FieldAndFormatProtoUtils.fromProto(fieldAndFormatProto); + + assertNotNull(result); + assertEquals("timestamp", result.field); + assertEquals("epoch_millis", result.format); + } +} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java index 11d3f058b7f35..92db0c7c51835 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java @@ -20,6 +20,7 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Set; @@ -109,7 +110,11 @@ public void testFromProtoWithDocValueFields() throws IOException { public void testFromProtoWithFetchFields() throws IOException { // Create a protobuf InnerHits with fetch fields - InnerHits innerHits = InnerHits.newBuilder().setName("test_inner_hits").addFields("field1").addFields("field2").build(); + InnerHits innerHits = InnerHits.newBuilder() + .setName("test_inner_hits") + .addFields(org.opensearch.protobufs.FieldAndFormat.newBuilder().setField("field1").build()) + .addFields(org.opensearch.protobufs.FieldAndFormat.newBuilder().setField("field2").build()) + .build(); // Call the method under test InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); @@ -220,8 +225,10 @@ public void testFromProtoWithMultipleInnerHits() throws IOException { List innerHitsList = Arrays.asList(innerHits1, innerHits2); - // Call the method under test - List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(innerHitsList); + List innerHitBuilders = new ArrayList<>(); + for (InnerHits innerHits : innerHitsList) { + innerHitBuilders.add(InnerHitsBuilderProtoUtils.fromProto(innerHits)); + } // Verify the result assertNotNull("InnerHitBuilder list should not be null", innerHitBuilders); @@ -239,8 +246,11 @@ public void testFromProtoWithMultipleInnerHits() throws IOException { } public void testFromProtoWithEmptyList() throws IOException { - // Call the method under test with an empty list - List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(Arrays.asList()); + List emptyList = Arrays.asList(); + List innerHitBuilders = new ArrayList<>(); + for (InnerHits innerHits : emptyList) { + innerHitBuilders.add(InnerHitsBuilderProtoUtils.fromProto(innerHits)); + } // Verify the result assertNotNull("InnerHitBuilder list should not be null", innerHitBuilders); @@ -257,16 +267,6 @@ public void testFromProtoWithNullInnerHits() { assertEquals("InnerHits cannot be null", exception.getMessage()); } - public void testFromProtoWithNullList() { - // Test null input validation for InnerHits list - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> InnerHitsBuilderProtoUtils.fromProto((List) null) - ); - - assertEquals("InnerHits list cannot be null", exception.getMessage()); - } - public void testFromProtoWithSort() throws IOException { // Create a protobuf InnerHits with sort (this will throw UnsupportedOperationException due to SortBuilderProtoUtils) InnerHits innerHits = InnerHits.newBuilder() diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverterTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000000..2326be024a0c2 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoConverterTests.java @@ -0,0 +1,262 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.transport.grpc.proto.request.search.query; + +import org.apache.lucene.search.join.ScoreMode; +import org.opensearch.index.query.NestedQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.ChildScoreMode; +import org.opensearch.protobufs.FieldValue; +import org.opensearch.protobufs.InnerHits; +import org.opensearch.protobufs.NestedQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.protobufs.TermQuery; +import org.opensearch.test.OpenSearchTestCase; + +public class NestedQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private NestedQueryBuilderProtoConverter converter; + + @Override + public void setUp() throws Exception { + super.setUp(); + converter = new NestedQueryBuilderProtoConverter(); + // Set up the registry with all built-in converters + QueryBuilderProtoConverterRegistryImpl registry = new QueryBuilderProtoConverterRegistryImpl(); + converter.setRegistry(registry); + } + + public void testGetHandledQueryCase() { + // Test that the converter returns the correct QueryContainerCase + assertEquals("Converter should handle NESTED case", QueryContainer.QueryContainerCase.NESTED, converter.getHandledQueryCase()); + } + + public void testFromProtoWithValidNestedQuery() { + // Create a nested query with term query as inner query + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MAX) + .setIgnoreUnmapped(true) + .setBoost(1.5f) + .setXName("test_nested_query") + .build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + QueryBuilder result = converter.fromProto(queryContainer); + + assertNotNull("Result should not be null", result); + assertTrue("Result should be NestedQueryBuilder", result instanceof NestedQueryBuilder); + + NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) result; + assertEquals("Path should match", "user", nestedQueryBuilder.path()); + assertEquals("Score mode should be Max", ScoreMode.Max, nestedQueryBuilder.scoreMode()); + assertTrue("Ignore unmapped should be true", nestedQueryBuilder.ignoreUnmapped()); + assertEquals("Boost should match", 1.5f, nestedQueryBuilder.boost(), 0.001f); + assertEquals("Query name should match", "test_nested_query", nestedQueryBuilder.queryName()); + } + + public void testFromProtoWithInnerHits() { + // Create inner hits + InnerHits innerHits = InnerHits.newBuilder().setName("user_inner_hits").setSize(10).setFrom(0).build(); + + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.age") + .setValue( + FieldValue.newBuilder() + .setGeneralNumber(org.opensearch.protobufs.GeneralNumber.newBuilder().setInt32Value(25).build()) + .build() + ) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).setInnerHits(innerHits).build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + QueryBuilder result = converter.fromProto(queryContainer); + + assertNotNull("Result should not be null", result); + assertTrue("Result should be NestedQueryBuilder", result instanceof NestedQueryBuilder); + + NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) result; + assertEquals("Path should match", "user", nestedQueryBuilder.path()); + assertNotNull("Inner hits should not be null", nestedQueryBuilder.innerHit()); + assertEquals("Inner hits name should match", "user_inner_hits", nestedQueryBuilder.innerHit().getName()); + assertEquals("Inner hits size should match", 10, nestedQueryBuilder.innerHit().getSize()); + } + + public void testFromProtoWithDifferentScoreModes() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Test AVG score mode + NestedQuery avgQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_AVG) + .build(); + + QueryContainer avgContainer = QueryContainer.newBuilder().setNested(avgQuery).build(); + NestedQueryBuilder avgResult = (NestedQueryBuilder) converter.fromProto(avgContainer); + assertEquals("Score mode should be Avg", ScoreMode.Avg, avgResult.scoreMode()); + + // Test MIN score mode + NestedQuery minQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MIN) + .build(); + + QueryContainer minContainer = QueryContainer.newBuilder().setNested(minQuery).build(); + NestedQueryBuilder minResult = (NestedQueryBuilder) converter.fromProto(minContainer); + assertEquals("Score mode should be Min", ScoreMode.Min, minResult.scoreMode()); + + // Test NONE score mode + NestedQuery noneQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_NONE) + .build(); + + QueryContainer noneContainer = QueryContainer.newBuilder().setNested(noneQuery).build(); + NestedQueryBuilder noneResult = (NestedQueryBuilder) converter.fromProto(noneContainer); + assertEquals("Score mode should be None", ScoreMode.None, noneResult.scoreMode()); + + // Test SUM score mode (maps to Total in Lucene) + NestedQuery sumQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_SUM) + .build(); + + QueryContainer sumContainer = QueryContainer.newBuilder().setNested(sumQuery).build(); + NestedQueryBuilder sumResult = (NestedQueryBuilder) converter.fromProto(sumContainer); + assertEquals("Score mode should be Total", ScoreMode.Total, sumResult.scoreMode()); + } + + public void testFromProtoWithNullQueryContainer() { + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(null)); + assertEquals("Exception message should match", "QueryContainer must contain a NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithWrongQueryType() { + // Create a term query instead of nested query + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(queryContainer)); + assertEquals("Exception message should match", "QueryContainer must contain a NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithEmptyQueryContainer() { + QueryContainer queryContainer = QueryContainer.newBuilder().build(); + + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(queryContainer)); + assertEquals("Exception message should match", "QueryContainer must contain a NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithMinimalNestedQuery() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.email") + .setValue(FieldValue.newBuilder().setString("test@example.com").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + QueryBuilder result = converter.fromProto(queryContainer); + + assertNotNull("Result should not be null", result); + assertTrue("Result should be NestedQueryBuilder", result instanceof NestedQueryBuilder); + + NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) result; + assertEquals("Path should match", "user", nestedQueryBuilder.path()); + assertEquals("Default score mode should be Avg", ScoreMode.Avg, nestedQueryBuilder.scoreMode()); + assertFalse("Ignore unmapped should be false by default", nestedQueryBuilder.ignoreUnmapped()); + assertEquals("Default boost should be 1.0", 1.0f, nestedQueryBuilder.boost(), 0.001f); + assertNull("Query name should be null by default", nestedQueryBuilder.queryName()); + } + + public void testFromProtoWithComplexNestedPath() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.profile.settings.theme") + .setValue(FieldValue.newBuilder().setString("dark").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user.profile.settings") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MAX) + .setIgnoreUnmapped(false) + .setBoost(2.5f) + .setXName("complex_nested_query") + .build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + QueryBuilder result = converter.fromProto(queryContainer); + + assertNotNull("Result should not be null", result); + assertTrue("Result should be NestedQueryBuilder", result instanceof NestedQueryBuilder); + + NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) result; + assertEquals("Path should match", "user.profile.settings", nestedQueryBuilder.path()); + assertEquals("Score mode should be Max", ScoreMode.Max, nestedQueryBuilder.scoreMode()); + assertFalse("Ignore unmapped should be false", nestedQueryBuilder.ignoreUnmapped()); + assertEquals("Boost should match", 2.5f, nestedQueryBuilder.boost(), 0.001f); + assertEquals("Query name should match", "complex_nested_query", nestedQueryBuilder.queryName()); + } + + public void testFromProtoWithUnspecifiedScoreMode() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.role") + .setValue(FieldValue.newBuilder().setString("admin").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_UNSPECIFIED) + .build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + QueryBuilder result = converter.fromProto(queryContainer); + + assertNotNull("Result should not be null", result); + assertTrue("Result should be NestedQueryBuilder", result instanceof NestedQueryBuilder); + + NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) result; + assertEquals("Default score mode should be Avg for unspecified", ScoreMode.Avg, nestedQueryBuilder.scoreMode()); + } +} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtilsTests.java new file mode 100644 index 0000000000000..9b5a2c524c980 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/NestedQueryBuilderProtoUtilsTests.java @@ -0,0 +1,488 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.transport.grpc.proto.request.search.query; + +import org.apache.lucene.search.join.ScoreMode; +import org.opensearch.index.query.NestedQueryBuilder; +import org.opensearch.protobufs.ChildScoreMode; +import org.opensearch.protobufs.FieldValue; +import org.opensearch.protobufs.InnerHits; +import org.opensearch.protobufs.NestedQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.protobufs.TermQuery; +import org.opensearch.test.OpenSearchTestCase; + +public class NestedQueryBuilderProtoUtilsTests extends OpenSearchTestCase { + + private QueryBuilderProtoConverterRegistryImpl registry; + + @Override + public void setUp() throws Exception { + super.setUp(); + // Set up the registry with all built-in converters + registry = new QueryBuilderProtoConverterRegistryImpl(); + } + + public void testFromProtoWithBasicNestedQuery() { + // Create a basic nested query with term query as inner query + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Path should match", "user", queryBuilder.path()); + assertNotNull("Inner query should not be null", queryBuilder.query()); + assertEquals("Default score mode should be Avg", ScoreMode.Avg, queryBuilder.scoreMode()); + } + + public void testFromProtoWithAllParameters() { + // Create inner hits + InnerHits innerHits = InnerHits.newBuilder().setName("user_hits").setSize(10).build(); + + // Create a term query as inner query + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.age") + .setValue( + FieldValue.newBuilder() + .setGeneralNumber(org.opensearch.protobufs.GeneralNumber.newBuilder().setInt32Value(25).build()) + .build() + ) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MAX) + .setIgnoreUnmapped(true) + .setBoost(2.0f) + .setXName("nested_user_query") + .setInnerHits(innerHits) + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Path should match", "user", queryBuilder.path()); + assertEquals("Score mode should be Max", ScoreMode.Max, queryBuilder.scoreMode()); + assertTrue("Ignore unmapped should be true", queryBuilder.ignoreUnmapped()); + assertEquals("Boost should match", 2.0f, queryBuilder.boost(), 0.001f); + assertEquals("Query name should match", "nested_user_query", queryBuilder.queryName()); + assertNotNull("Inner hits should not be null", queryBuilder.innerHit()); + } + + public void testFromProtoWithDifferentScoreModes() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Test AVG score mode + NestedQuery avgQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_AVG) + .build(); + + NestedQueryBuilder avgBuilder = NestedQueryBuilderProtoUtils.fromProto(avgQuery, registry); + assertEquals("Score mode should be Avg", ScoreMode.Avg, avgBuilder.scoreMode()); + + // Test MIN score mode + NestedQuery minQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MIN) + .build(); + + NestedQueryBuilder minBuilder = NestedQueryBuilderProtoUtils.fromProto(minQuery, registry); + assertEquals("Score mode should be Min", ScoreMode.Min, minBuilder.scoreMode()); + + // Test NONE score mode + NestedQuery noneQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_NONE) + .build(); + + NestedQueryBuilder noneBuilder = NestedQueryBuilderProtoUtils.fromProto(noneQuery, registry); + assertEquals("Score mode should be None", ScoreMode.None, noneBuilder.scoreMode()); + + // Test SUM score mode (maps to Total in Lucene) + NestedQuery sumQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_SUM) + .build(); + + NestedQueryBuilder sumBuilder = NestedQueryBuilderProtoUtils.fromProto(sumQuery, registry); + assertEquals("Score mode should be Total", ScoreMode.Total, sumBuilder.scoreMode()); + } + + public void testFromProtoWithUnspecifiedScoreMode() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.role") + .setValue(FieldValue.newBuilder().setString("admin").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_UNSPECIFIED) + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + assertEquals("Default score mode should be Avg for unspecified", ScoreMode.Avg, queryBuilder.scoreMode()); + } + + public void testFromProtoWithNullNestedQuery() { + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(null, registry) + ); + assertEquals("Exception message should match", "NestedQuery cannot be null", exception.getMessage()); + } + + public void testFromProtoWithEmptyPath() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("").setQuery(innerQueryContainer).build(); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertEquals("Exception message should match", "Path is required for NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithNullPath() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setQuery(innerQueryContainer).build(); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertEquals("Exception message should match", "Path is required for NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithMissingQuery() { + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").build(); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertEquals("Exception message should match", "Query is required for NestedQuery", exception.getMessage()); + } + + public void testFromProtoWithOptionalParametersOnly() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.email") + .setValue(FieldValue.newBuilder().setString("test@example.com").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setIgnoreUnmapped(false) + .setBoost(1.5f) + .setXName("optional_params_query") + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Path should match", "user", queryBuilder.path()); + assertFalse("Ignore unmapped should be false", queryBuilder.ignoreUnmapped()); + assertEquals("Boost should match", 1.5f, queryBuilder.boost(), 0.001f); + assertEquals("Query name should match", "optional_params_query", queryBuilder.queryName()); + assertEquals("Default score mode should be Avg", ScoreMode.Avg, queryBuilder.scoreMode()); + } + + public void testFromProtoWithInnerHitsOnly() { + InnerHits innerHits = InnerHits.newBuilder().setName("product_hits").setSize(5).setFrom(0).build(); + + TermQuery termQuery = TermQuery.newBuilder() + .setField("products.category") + .setValue(FieldValue.newBuilder().setString("electronics").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("products") + .setQuery(innerQueryContainer) + .setInnerHits(innerHits) + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Path should match", "products", queryBuilder.path()); + assertNotNull("Inner hits should not be null", queryBuilder.innerHit()); + assertEquals("Inner hits name should match", "product_hits", queryBuilder.innerHit().getName()); + assertEquals("Inner hits size should match", 5, queryBuilder.innerHit().getSize()); + } + + public void testFromProtoWithComplexNestedPath() { + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.profile.settings.theme") + .setValue(FieldValue.newBuilder().setString("dark").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user.profile") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MAX) + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Path should match", "user.profile", queryBuilder.path()); + assertEquals("Score mode should be Max", ScoreMode.Max, queryBuilder.scoreMode()); + } + + public void testFromProtoWithInvalidInnerQuery() { + // Create an empty QueryContainer to simulate invalid query conversion + QueryContainer emptyQueryContainer = QueryContainer.newBuilder().build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(emptyQueryContainer).build(); + + // This should throw an exception when trying to convert the inner query + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertTrue( + "Exception message should contain 'Failed to convert inner query'", + exception.getMessage().contains("Failed to convert inner query for NestedQuery") + ); + } + + public void testFromProtoWithInvalidInnerHits() { + // Test with invalid inner hits to trigger exception handling + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Create inner hits with invalid configuration that will cause conversion to fail + InnerHits invalidInnerHits = InnerHits.newBuilder() + .setName("valid_name") + .setSize(-1) // Invalid size - should trigger exception + .build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setInnerHits(invalidInnerHits) + .build(); + + // This should throw an exception due to invalid inner hits configuration + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertTrue( + "Exception message should contain 'Failed to convert inner hits'", + exception.getMessage().contains("Failed to convert inner hits for NestedQuery") + ); + } + + public void testParseScoreModeDefaultCase() { + // Test when no score mode is set - should default to ScoreMode.Avg + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Don't set score mode - this will test the default behavior + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + // No setScoreMode() call - should use default ScoreMode.Avg + .build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + assertEquals("Default score mode should be Avg when not set", ScoreMode.Avg, queryBuilder.scoreMode()); + } + + public void testFromProtoWithIgnoreUnmappedTrue() { + // Test setting ignoreUnmapped to true + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).setIgnoreUnmapped(true).build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + assertTrue("Ignore unmapped should be true", queryBuilder.ignoreUnmapped()); + } + + public void testFromProtoWithIgnoreUnmappedFalse() { + // Test setting ignoreUnmapped to false + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).setIgnoreUnmapped(false).build(); + + NestedQueryBuilder queryBuilder = NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry); + assertFalse("Ignore unmapped should be false", queryBuilder.ignoreUnmapped()); + } + + public void testFromProtoWithAllScoreModes() { + // Test all score modes to achieve full coverage of parseScoreMode method + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.status") + .setValue(FieldValue.newBuilder().setString("active").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Test MAX score mode + NestedQuery maxQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MAX) + .build(); + + NestedQueryBuilder maxBuilder = NestedQueryBuilderProtoUtils.fromProto(maxQuery, registry); + assertEquals("Score mode should be Max", ScoreMode.Max, maxBuilder.scoreMode()); + + // Test MIN score mode + NestedQuery minQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_MIN) + .build(); + + NestedQueryBuilder minBuilder = NestedQueryBuilderProtoUtils.fromProto(minQuery, registry); + assertEquals("Score mode should be Min", ScoreMode.Min, minBuilder.scoreMode()); + + // Test NONE score mode + NestedQuery noneQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_NONE) + .build(); + + NestedQueryBuilder noneBuilder = NestedQueryBuilderProtoUtils.fromProto(noneQuery, registry); + assertEquals("Score mode should be None", ScoreMode.None, noneBuilder.scoreMode()); + + // Test SUM score mode (maps to Total in Lucene) + NestedQuery sumQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_SUM) + .build(); + + NestedQueryBuilder sumBuilder = NestedQueryBuilderProtoUtils.fromProto(sumQuery, registry); + assertEquals("Score mode should be Total", ScoreMode.Total, sumBuilder.scoreMode()); + } + + public void testFromProtoWithInvalidQueryConversion() { + // Test exception handling when inner query conversion fails + // Create a mock registry that throws an exception + QueryBuilderProtoConverterRegistryImpl mockRegistry = new QueryBuilderProtoConverterRegistryImpl() { + @Override + public org.opensearch.index.query.QueryBuilder fromProto(QueryContainer queryContainer) { + throw new RuntimeException("Simulated conversion failure"); + } + }; + + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder().setPath("user").setQuery(innerQueryContainer).build(); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, mockRegistry) + ); + assertTrue( + "Exception message should contain 'Failed to convert inner query'", + exception.getMessage().contains("Failed to convert inner query for NestedQuery") + ); + assertTrue("Exception message should contain the cause", exception.getMessage().contains("Simulated conversion failure")); + } + + public void testFromProtoWithInvalidInnerHitsConversion() { + // Test exception handling when inner hits conversion fails + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Create inner hits that will cause conversion to fail + // Using an invalid size that should trigger an exception in InnerHitsBuilderProtoUtils + org.opensearch.protobufs.InnerHits invalidInnerHits = org.opensearch.protobufs.InnerHits.newBuilder() + .setName("test_inner_hits") + .setSize(-1) // Invalid size - should trigger exception + .build(); + + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setInnerHits(invalidInnerHits) + .build(); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> NestedQueryBuilderProtoUtils.fromProto(nestedQuery, registry) + ); + assertTrue( + "Exception message should contain 'Failed to convert inner hits'", + exception.getMessage().contains("Failed to convert inner hits for NestedQuery") + ); + } +} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java index 6c2e7aab1f1c1..b4d685d560f81 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java @@ -9,6 +9,7 @@ import org.opensearch.index.query.QueryBuilder; import org.opensearch.protobufs.BoolQuery; +import org.opensearch.protobufs.ChildScoreMode; import org.opensearch.protobufs.CoordsGeoBounds; import org.opensearch.protobufs.DoubleArray; import org.opensearch.protobufs.ExistsQuery; @@ -23,6 +24,7 @@ import org.opensearch.protobufs.MatchPhraseQuery; import org.opensearch.protobufs.MinimumShouldMatch; import org.opensearch.protobufs.MultiMatchQuery; +import org.opensearch.protobufs.NestedQuery; import org.opensearch.protobufs.QueryContainer; import org.opensearch.protobufs.RegexpQuery; import org.opensearch.protobufs.Script; @@ -94,6 +96,34 @@ public void testScriptQueryConversion() { assertEquals("Should be a ScriptQueryBuilder", "org.opensearch.index.query.ScriptQueryBuilder", queryBuilder.getClass().getName()); } + public void testNestedQueryConversion() { + // Create a Term query as inner query for the nested query + TermQuery termQuery = TermQuery.newBuilder() + .setField("user.name") + .setValue(FieldValue.newBuilder().setString("john").build()) + .build(); + + QueryContainer innerQueryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Create a Nested query container + NestedQuery nestedQuery = NestedQuery.newBuilder() + .setPath("user") + .setQuery(innerQueryContainer) + .setScoreMode(ChildScoreMode.CHILD_SCORE_MODE_AVG) + .setBoost(1.5f) + .setXName("test_nested_query") + .build(); + + QueryContainer queryContainer = QueryContainer.newBuilder().setNested(nestedQuery).build(); + + // Convert using the registry + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Should be a NestedQueryBuilder", "org.opensearch.index.query.NestedQueryBuilder", queryBuilder.getClass().getName()); + } + public void testNullQueryContainer() { expectThrows(IllegalArgumentException.class, () -> registry.fromProto(null)); } @@ -373,4 +403,108 @@ public void testBoolQueryConversion() { assertNotNull("QueryBuilder should not be null", queryBuilder); assertEquals("Should be a BoolQueryBuilder", "org.opensearch.index.query.BoolQueryBuilder", queryBuilder.getClass().getName()); } + + public void testRegisterConverter() { + // Create a custom converter for testing + QueryBuilderProtoConverter customConverter = new QueryBuilderProtoConverter() { + @Override + public void setRegistry(org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry registry) { + // No-op for test + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.MATCH_ALL; // Use existing case for simplicity + } + + @Override + public org.opensearch.index.query.QueryBuilder fromProto(QueryContainer queryContainer) { + // Return a simple match all query for testing + return new org.opensearch.index.query.MatchAllQueryBuilder(); + } + }; + + // Test that registerConverter method can be called without throwing exceptions + try { + registry.registerConverter(customConverter); + // If we reach here, no exception was thrown + } catch (Exception e) { + fail("registerConverter should not throw an exception: " + e.getMessage()); + } + + // Verify that the registry still works after adding a converter + QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(MatchAllQuery.newBuilder().build()).build(); + + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + assertNotNull("QueryBuilder should not be null after registering custom converter", queryBuilder); + } + + public void testUpdateRegistryOnAllConverters() { + // Test that updateRegistryOnAllConverters method can be called without throwing exceptions + try { + registry.updateRegistryOnAllConverters(); + // If we reach here, no exception was thrown + } catch (Exception e) { + fail("updateRegistryOnAllConverters should not throw an exception: " + e.getMessage()); + } + + // Verify that the registry still works after updating all converters + QueryContainer queryContainer = QueryContainer.newBuilder() + .setTerm( + TermQuery.newBuilder().setField("test_field").setValue(FieldValue.newBuilder().setString("test_value").build()).build() + ) + .build(); + + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + assertNotNull("QueryBuilder should not be null after updating registry on all converters", queryBuilder); + assertEquals("Should be a TermQueryBuilder", "org.opensearch.index.query.TermQueryBuilder", queryBuilder.getClass().getName()); + } + + public void testRegisterConverterAndUpdateRegistry() { + // Test the combination of registering a converter and then updating the registry + QueryBuilderProtoConverter customConverter = new QueryBuilderProtoConverter() { + private org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry registryRef; + + @Override + public void setRegistry(org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry registry) { + this.registryRef = registry; + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.EXISTS; // Use existing case + } + + @Override + public org.opensearch.index.query.QueryBuilder fromProto(QueryContainer queryContainer) { + // Return an exists query for testing + return new org.opensearch.index.query.ExistsQueryBuilder("test_field"); + } + }; + + // Register the custom converter + try { + registry.registerConverter(customConverter); + // If we reach here, no exception was thrown + } catch (Exception e) { + fail("registerConverter should not throw an exception: " + e.getMessage()); + } + + // Update registry on all converters + try { + registry.updateRegistryOnAllConverters(); + // If we reach here, no exception was thrown + } catch (Exception e) { + fail("updateRegistryOnAllConverters should not throw an exception: " + e.getMessage()); + } + + // Verify that the registry still works after both operations + QueryContainer queryContainer = QueryContainer.newBuilder() + .setExists(ExistsQuery.newBuilder().setField("test_field").build()) + .build(); + + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + assertNotNull("QueryBuilder should not be null after register and update operations", queryBuilder); + assertEquals("Should be an ExistsQueryBuilder", "org.opensearch.index.query.ExistsQueryBuilder", queryBuilder.getClass().getName()); + } } diff --git a/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java index a2e43988c0937..e3294ebc4376f 100644 --- a/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java @@ -103,7 +103,7 @@ public NestedQueryBuilder(String path, QueryBuilder query, ScoreMode scoreMode) this(path, query, scoreMode, null); } - private NestedQueryBuilder(String path, QueryBuilder query, ScoreMode scoreMode, InnerHitBuilder innerHitBuilder) { + public NestedQueryBuilder(String path, QueryBuilder query, ScoreMode scoreMode, InnerHitBuilder innerHitBuilder) { this.path = requireValue(path, "[" + NAME + "] requires 'path' field"); this.query = requireValue(query, "[" + NAME + "] requires 'query' field"); this.scoreMode = requireValue(scoreMode, "[" + NAME + "] requires 'score_mode' field");