diff --git a/CHANGELOG.md b/CHANGELOG.md index bd9ec337e9449..02c45d0e36057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Star-Tree] Add star-tree search related stats ([#18707](https://github.com/opensearch-project/OpenSearch/pull/18707)) - Add support for plugins to profile information ([#18656](https://github.com/opensearch-project/OpenSearch/pull/18656)) - Add support for Combined Fields query ([#18724](https://github.com/opensearch-project/OpenSearch/pull/18724)) +- Make GRPC transport extensible to allow plugins to register and expose their own GRPC services ([#18516](https://github.com/opensearch-project/OpenSearch/pull/18516)) - Added approximation support for range queries with now in date field ([#18511](https://github.com/opensearch-project/OpenSearch/pull/18511)) ### Changed diff --git a/plugins/transport-grpc/build.gradle b/plugins/transport-grpc/build.gradle index 5553fdca716ff..5920ba83b6d0d 100644 --- a/plugins/transport-grpc/build.gradle +++ b/plugins/transport-grpc/build.gradle @@ -35,7 +35,7 @@ dependencies { implementation "io.grpc:grpc-stub:${versions.grpc}" implementation "io.grpc:grpc-util:${versions.grpc}" implementation "io.perfmark:perfmark-api:0.27.0" - implementation "org.opensearch:protobufs:0.3.0" + implementation "org.opensearch:protobufs:0.4.0" testImplementation project(':test:framework') } diff --git a/plugins/transport-grpc/licenses/protobufs-0.3.0.jar.sha1 b/plugins/transport-grpc/licenses/protobufs-0.3.0.jar.sha1 deleted file mode 100644 index 319d6aa6545c2..0000000000000 --- a/plugins/transport-grpc/licenses/protobufs-0.3.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -5e22ed37e4535c9c9cfeb8993f5294ba1201795c \ No newline at end of file diff --git a/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 b/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 new file mode 100644 index 0000000000000..12f76199639f8 --- /dev/null +++ b/plugins/transport-grpc/licenses/protobufs-0.4.0.jar.sha1 @@ -0,0 +1 @@ +af2d6818dab60d54689122e57f3d3b8fb86cf67b \ No newline at end of file diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/GrpcPlugin.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/GrpcPlugin.java index a809092c92673..cf64e07f004ea 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/GrpcPlugin.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/GrpcPlugin.java @@ -18,9 +18,13 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoConverterRegistry; import org.opensearch.plugin.transport.grpc.services.DocumentServiceImpl; import org.opensearch.plugin.transport.grpc.services.SearchServiceImpl; import org.opensearch.plugin.transport.grpc.ssl.SecureNetty4GrpcServerTransport; +import org.opensearch.plugins.ExtensiblePlugin; import org.opensearch.plugins.NetworkPlugin; import org.opensearch.plugins.Plugin; import org.opensearch.plugins.SecureAuxTransportSettingsProvider; @@ -32,6 +36,7 @@ import org.opensearch.transport.client.Client; import org.opensearch.watcher.ResourceWatcherService; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -55,15 +60,55 @@ /** * Main class for the gRPC plugin. */ -public final class GrpcPlugin extends Plugin implements NetworkPlugin { +public final class GrpcPlugin extends Plugin implements NetworkPlugin, ExtensiblePlugin { private Client client; + private final List queryConverters = new ArrayList<>(); + private QueryBuilderProtoConverterRegistry queryRegistry; + private AbstractQueryBuilderProtoUtils queryUtils; /** * Creates a new GrpcPlugin instance. */ public GrpcPlugin() {} + /** + * Loads extensions from other plugins. + * This method is called by the OpenSearch plugin system to load extensions from other plugins. + * + * @param loader The extension loader to use for loading extensions + */ + @Override + public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { + // Load query converters from other plugins + List extensions = loader.loadExtensions(QueryBuilderProtoConverter.class); + if (extensions != null) { + queryConverters.addAll(extensions); + } + } + + /** + * Get the list of query converters, including those loaded from extensions. + * + * @return The list of query converters + */ + public List getQueryConverters() { + return Collections.unmodifiableList(queryConverters); + } + + /** + * Get the query utils instance. + * + * @return The query utils instance + * @throws IllegalStateException if queryUtils is not initialized + */ + public AbstractQueryBuilderProtoUtils getQueryUtils() { + if (queryUtils == null) { + throw new IllegalStateException("Query utils not initialized. Make sure createComponents has been called."); + } + return queryUtils; + } + /** * Provides auxiliary transports for the plugin. * Creates and returns a map of transport names to transport suppliers. @@ -75,6 +120,7 @@ public GrpcPlugin() {} * @param clusterSettings The cluster settings * @param tracer The tracer * @return A map of transport names to transport suppliers + * @throws IllegalStateException if queryRegistry is not initialized */ @Override public Map> getAuxTransports( @@ -88,7 +134,15 @@ public Map> getAuxTransports( if (client == null) { throw new RuntimeException("client cannot be null"); } - List grpcServices = registerGRPCServices(new DocumentServiceImpl(client), new SearchServiceImpl(client)); + + if (queryRegistry == null) { + throw new IllegalStateException("createComponents must be called before getAuxTransports to initialize the registry"); + } + + List grpcServices = registerGRPCServices( + new DocumentServiceImpl(client), + new SearchServiceImpl(client, queryUtils) + ); AuxTransport transport = new Netty4GrpcServerTransport(settings, grpcServices, networkService); return Collections.singletonMap(transport.settingKey(), () -> transport); } @@ -106,6 +160,7 @@ public Map> getAuxTransports( * @param tracer The tracer * @param secureAuxTransportSettingsProvider provides ssl context params * @return A map of transport names to transport suppliers + * @throws IllegalStateException if queryRegistry is not initialized */ @Override public Map> getSecureAuxTransports( @@ -120,7 +175,15 @@ public Map> getSecureAuxTransports( if (client == null) { throw new RuntimeException("client cannot be null"); } - List grpcServices = registerGRPCServices(new DocumentServiceImpl(client), new SearchServiceImpl(client)); + + if (queryRegistry == null) { + throw new IllegalStateException("createComponents must be called before getSecureAuxTransports to initialize the registry"); + } + + List grpcServices = registerGRPCServices( + new DocumentServiceImpl(client), + new SearchServiceImpl(client, queryUtils) + ); AuxTransport transport = new SecureNetty4GrpcServerTransport( settings, grpcServices, @@ -164,7 +227,7 @@ public List> getSettings() { /** * Creates components used by the plugin. - * Stores the client for later use in creating gRPC services. + * Stores the client for later use in creating gRPC services, and the query registry which registers the types of supported GRPC Search queries. * * @param client The client * @param clusterService The cluster service @@ -195,6 +258,17 @@ public Collection createComponents( ) { this.client = client; + // Create the registry + this.queryRegistry = new QueryBuilderProtoConverterRegistry(); + + // Create the query utils instance + this.queryUtils = new AbstractQueryBuilderProtoUtils(queryRegistry); + + // Register external converters + for (QueryBuilderProtoConverter converter : queryConverters) { + queryRegistry.registerConverter(converter); + } + return super.createComponents( client, clusterService, diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtils.java index d8765d59d604c..e91cfcfacc307 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtils.java @@ -16,6 +16,7 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.QueryBuilder; import org.opensearch.plugin.transport.grpc.proto.request.common.FetchSourceContextProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; import org.opensearch.plugin.transport.grpc.proto.request.search.suggest.TermSuggestionBuilderProtoUtils; import org.opensearch.protobufs.SearchRequest; import org.opensearch.protobufs.SearchRequestBody; @@ -40,9 +41,7 @@ import static org.opensearch.search.suggest.SuggestBuilders.termSuggestion; /** - * Utility class for converting SearchRequest objects between OpenSearch and Protocol Buffers formats. - * This class provides methods to prepare, parse, and transform search requests to ensure proper - * communication between gRPC clients and the OpenSearch server. + * Utility class for converting SearchRequest Protocol Buffers to objects */ public class SearchRequestProtoUtils { @@ -58,11 +57,16 @@ private SearchRequestProtoUtils() { * * @param request the Protocol Buffer SearchRequest to execute * @param client the client to use for execution + * @param queryUtils the query utils instance for parsing queries * @return the SearchRequest to execute * @throws IOException if an I/O exception occurred parsing the request and preparing for * execution */ - public static org.opensearch.action.search.SearchRequest prepareRequest(SearchRequest request, Client client) throws IOException { + public static org.opensearch.action.search.SearchRequest prepareRequest( + org.opensearch.protobufs.SearchRequest request, + Client client, + AbstractQueryBuilderProtoUtils queryUtils + ) throws IOException { org.opensearch.action.search.SearchRequest searchRequest = new org.opensearch.action.search.SearchRequest(); /* @@ -79,26 +83,28 @@ public static org.opensearch.action.search.SearchRequest prepareRequest(SearchRe */ IntConsumer setSize = size -> searchRequest.source().size(size); // TODO avoid hidden cast to NodeClient here - parseSearchRequest(searchRequest, request, ((NodeClient) client).getNamedWriteableRegistry(), setSize); + parseSearchRequest(searchRequest, request, ((NodeClient) client).getNamedWriteableRegistry(), setSize, queryUtils); return searchRequest; } /** * Parses a protobuf {@link org.opensearch.protobufs.SearchRequest} to a {@link org.opensearch.action.search.SearchRequest}. * This method is similar to the logic in {@link RestSearchAction#parseSearchRequest(org.opensearch.action.search.SearchRequest, RestRequest, XContentParser, NamedWriteableRegistry, IntConsumer)} - * Specifically, this method handles the URL parameters, and internally calls {@link SearchSourceBuilderProtoUtils#parseProto(SearchSourceBuilder, SearchRequestBody)} + * Specifically, this method handles the URL parameters, and internally calls {@link SearchSourceBuilderProtoUtils#parseProto(SearchSourceBuilder, SearchRequestBody, AbstractQueryBuilderProtoUtils)} * * @param searchRequest the SearchRequest to populate * @param request the Protocol Buffer SearchRequest to parse * @param namedWriteableRegistry the registry for named writeables * @param setSize consumer for setting the size parameter + * @param queryUtils the query utils instance for parsing queries * @throws IOException if an I/O exception occurred during parsing */ protected static void parseSearchRequest( org.opensearch.action.search.SearchRequest searchRequest, org.opensearch.protobufs.SearchRequest request, NamedWriteableRegistry namedWriteableRegistry, - IntConsumer setSize + IntConsumer setSize, + AbstractQueryBuilderProtoUtils queryUtils ) throws IOException { if (searchRequest.source() == null) { searchRequest.source(new SearchSourceBuilder()); @@ -110,7 +116,7 @@ protected static void parseSearchRequest( } searchRequest.indices(indexArr); - SearchSourceBuilderProtoUtils.parseProto(searchRequest.source(), request.getRequestBody()); + SearchSourceBuilderProtoUtils.parseProto(searchRequest.source(), request.getRequestBody(), queryUtils); final int batchedReduceSize = request.hasBatchedReduceSize() ? request.getBatchedReduceSize() diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java index d2c010dcb15fc..c89f99f0bd93c 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java @@ -42,14 +42,35 @@ private SearchSourceBuilderProtoUtils() { } /** - * Parses a protobuf SearchRequestBody into a SearchSourceBuilder. + * Parses a protobuf SearchRequestBody into a SearchSourceBuilder using an instance-based query utils. * This method is equivalent to {@link SearchSourceBuilder#parseXContent(XContentParser, boolean)} * * @param searchSourceBuilder The SearchSourceBuilder to populate * @param protoRequest The Protocol Buffer SearchRequest to parse + * @param queryUtils The query utils instance to use for parsing queries * @throws IOException if there's an error during parsing */ - protected static void parseProto(SearchSourceBuilder searchSourceBuilder, SearchRequestBody protoRequest) throws IOException { + public static void parseProto( + SearchSourceBuilder searchSourceBuilder, + SearchRequestBody protoRequest, + AbstractQueryBuilderProtoUtils queryUtils + ) throws IOException { + // Parse all non-query fields + parseNonQueryFields(searchSourceBuilder, protoRequest); + + // Handle queries using the instance-based approach + if (protoRequest.hasQuery()) { + searchSourceBuilder.query(queryUtils.parseInnerQueryBuilderProto(protoRequest.getQuery())); + } + if (protoRequest.hasPostFilter()) { + searchSourceBuilder.postFilter(queryUtils.parseInnerQueryBuilderProto(protoRequest.getPostFilter())); + } + } + + /** + * Parses all fields except queries from the protobuf SearchRequestBody. + */ + private static void parseNonQueryFields(SearchSourceBuilder searchSourceBuilder, SearchRequestBody protoRequest) throws IOException { // TODO what to do about parser.getDeprecationHandler() for protos? if (protoRequest.hasFrom()) { @@ -111,12 +132,6 @@ protected static void parseProto(SearchSourceBuilder searchSourceBuilder, Search if (protoRequest.hasVerbosePipeline()) { searchSourceBuilder.verbosePipeline(protoRequest.getVerbosePipeline()); } - if (protoRequest.hasQuery()) { - searchSourceBuilder.query(AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(protoRequest.getQuery())); - } - if (protoRequest.hasPostFilter()) { - searchSourceBuilder.postFilter(AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(protoRequest.getPostFilter())); - } if (protoRequest.hasSource()) { searchSourceBuilder.fetchSource(FetchSourceContextProtoUtils.fromProto(protoRequest.getSource())); } diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtils.java index a0bfbc70313a1..0ce89d5e9c9b1 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtils.java @@ -19,8 +19,19 @@ */ public class AbstractQueryBuilderProtoUtils { - private AbstractQueryBuilderProtoUtils() { - // Utility class, no instances + private final QueryBuilderProtoConverterRegistry registry; + + /** + * Creates a new instance with the specified registry. + * + * @param registry The registry to use for query conversion + * @throws IllegalArgumentException if registry is null + */ + public AbstractQueryBuilderProtoUtils(QueryBuilderProtoConverterRegistry registry) { + if (registry == null) { + throw new IllegalArgumentException("Registry cannot be null"); + } + this.registry = registry; } /** @@ -33,23 +44,12 @@ private AbstractQueryBuilderProtoUtils() { * @return A QueryBuilder instance configured according to the input query parameters * @throws UnsupportedOperationException if the query type is not supported */ - public static QueryBuilder parseInnerQueryBuilderProto(QueryContainer queryContainer) throws UnsupportedOperationException { - QueryBuilder result; - - if (queryContainer.hasMatchAll()) { - result = MatchAllQueryBuilderProtoUtils.fromProto(queryContainer.getMatchAll()); - } else if (queryContainer.hasMatchNone()) { - result = MatchNoneQueryBuilderProtoUtils.fromProto(queryContainer.getMatchNone()); - } else if (queryContainer.getTermCount() > 0) { - result = TermQueryBuilderProtoUtils.fromProto(queryContainer.getTermMap()); - } else if (queryContainer.hasTerms()) { - result = TermsQueryBuilderProtoUtils.fromProto(queryContainer.getTerms()); - } - // TODO add more query types - else { - throw new UnsupportedOperationException("Search query type not supported yet."); + public QueryBuilder parseInnerQueryBuilderProto(QueryContainer queryContainer) throws UnsupportedOperationException { + // Validate input + if (queryContainer == null) { + throw new IllegalArgumentException("Query container cannot be null"); } - return result; + return registry.fromProto(queryContainer); } } diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverter.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..c74da63e7cd93 --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverter.java @@ -0,0 +1,40 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +/** + * Converter for MatchAll queries. + * This class implements the QueryBuilderProtoConverter interface to provide MatchAll query support + * for the gRPC transport plugin. + */ +public class MatchAllQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + /** + * Constructs a new MatchAllQueryBuilderProtoConverter. + */ + public MatchAllQueryBuilderProtoConverter() { + // Default constructor + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.MATCH_ALL; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || !queryContainer.hasMatchAll()) { + throw new IllegalArgumentException("QueryContainer does not contain a MatchAll query"); + } + + return MatchAllQueryBuilderProtoUtils.fromProto(queryContainer.getMatchAll()); + } +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverter.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..1742768bcd471 --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverter.java @@ -0,0 +1,40 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +/** + * Converter for MatchNone queries. + * This class implements the QueryBuilderProtoConverter interface to provide MatchNone query support + * for the gRPC transport plugin. + */ +public class MatchNoneQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + /** + * Constructs a new MatchNoneQueryBuilderProtoConverter. + */ + public MatchNoneQueryBuilderProtoConverter() { + // Default constructor + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.MATCH_NONE; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || !queryContainer.hasMatchNone()) { + throw new IllegalArgumentException("QueryContainer does not contain a MatchNone query"); + } + + return MatchNoneQueryBuilderProtoUtils.fromProto(queryContainer.getMatchNone()); + } +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverter.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..02b90e287b115 --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverter.java @@ -0,0 +1,34 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +/** + * Interface for converting protobuf query messages to OpenSearch QueryBuilder objects. + * External plugins can implement this interface to provide their own query types. + */ +public interface QueryBuilderProtoConverter { + + /** + * Returns the QueryContainerCase this converter handles. + * + * @return The QueryContainerCase + */ + QueryContainer.QueryContainerCase getHandledQueryCase(); + + /** + * Converts a protobuf query container to an OpenSearch QueryBuilder. + * + * @param queryContainer The protobuf query container + * @return The corresponding OpenSearch QueryBuilder + * @throws IllegalArgumentException if the query cannot be converted + */ + QueryBuilder fromProto(QueryContainer queryContainer); +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistry.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistry.java new file mode 100644 index 0000000000000..fcb5ab3fdfbe3 --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistry.java @@ -0,0 +1,142 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.inject.Singleton; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.ServiceLoader; + +/** + * Registry for QueryBuilderProtoConverter implementations. + * This class discovers and manages all available converters. + */ +@Singleton +public class QueryBuilderProtoConverterRegistry { + + private static final Logger logger = LogManager.getLogger(QueryBuilderProtoConverterRegistry.class); + private final Map converterMap = new HashMap<>(); + + /** + * Creates a new registry and loads all available converters. + */ + @Inject + public QueryBuilderProtoConverterRegistry() { + // Load built-in converters + registerBuiltInConverters(); + + // Discover converters from other plugins using Java's ServiceLoader + loadExternalConverters(); + } + + /** + * Registers the built-in converters. + * Protected for testing purposes. + */ + protected void registerBuiltInConverters() { + // Add built-in converters + registerConverter(new MatchAllQueryBuilderProtoConverter()); + registerConverter(new MatchNoneQueryBuilderProtoConverter()); + registerConverter(new TermQueryBuilderProtoConverter()); + registerConverter(new TermsQueryBuilderProtoConverter()); + + logger.info("Registered {} built-in query converters", converterMap.size()); + } + + /** + * Loads external converters using Java's ServiceLoader mechanism. + * Protected for testing purposes. + */ + protected void loadExternalConverters() { + ServiceLoader serviceLoader = ServiceLoader.load(QueryBuilderProtoConverter.class); + Iterator iterator = serviceLoader.iterator(); + + int count = 0; + int failedCount = 0; + while (iterator.hasNext()) { + try { + QueryBuilderProtoConverter converter = iterator.next(); + registerConverter(converter); + count++; + logger.info("Loaded external query converter for {}: {}", converter.getHandledQueryCase(), converter.getClass().getName()); + } catch (Exception e) { + failedCount++; + logger.error("Failed to load external query converter", e); + } + } + + logger.info("Loaded {} external query converters ({} failed)", count, failedCount); + } + + /** + * Converts a protobuf query container to an OpenSearch QueryBuilder. + * + * @param queryContainer The protobuf query container + * @return The corresponding OpenSearch QueryBuilder + * @throws IllegalArgumentException if no converter can handle the query + */ + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null) { + throw new IllegalArgumentException("Query container cannot be null"); + } + + // Use direct map lookup for better performance + QueryContainer.QueryContainerCase queryCase = queryContainer.getQueryContainerCase(); + QueryBuilderProtoConverter converter = converterMap.get(queryCase); + + if (converter != null) { + logger.debug("Using converter for {}: {}", queryCase, converter.getClass().getName()); + return converter.fromProto(queryContainer); + } + + throw new IllegalArgumentException("Unsupported query type in container: " + queryContainer + " (case: " + queryCase + ")"); + } + + /** + * Registers a new converter. + * + * @param converter The converter to register + * @throws IllegalArgumentException if the converter is null or its handled query case is invalid + */ + public void registerConverter(QueryBuilderProtoConverter converter) { + if (converter == null) { + throw new IllegalArgumentException("Converter cannot be null"); + } + + QueryContainer.QueryContainerCase queryCase = converter.getHandledQueryCase(); + + if (queryCase == null) { + throw new IllegalArgumentException("Handled query case cannot be null for converter: " + converter.getClass().getName()); + } + + if (queryCase == QueryContainer.QueryContainerCase.QUERYCONTAINER_NOT_SET) { + throw new IllegalArgumentException( + "Cannot register converter for QUERYCONTAINER_NOT_SET case: " + converter.getClass().getName() + ); + } + + QueryBuilderProtoConverter existingConverter = converterMap.put(queryCase, converter); + if (existingConverter != null) { + logger.warn( + "Replacing existing converter for query type {}: {} -> {}", + queryCase, + existingConverter.getClass().getName(), + converter.getClass().getName() + ); + } + + logger.debug("Registered query converter for {}: {}", queryCase, converter.getClass().getName()); + } +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverter.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..55b0e3a39954f --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverter.java @@ -0,0 +1,40 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +/** + * Converter for Term queries. + * This class implements the QueryBuilderProtoConverter interface to provide Term query support + * for the gRPC transport plugin. + */ +public class TermQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + /** + * Constructs a new TermQueryBuilderProtoConverter. + */ + public TermQueryBuilderProtoConverter() { + // Default constructor + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.TERM; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || queryContainer.getQueryContainerCase() != QueryContainer.QueryContainerCase.TERM) { + throw new IllegalArgumentException("QueryContainer does not contain a Term query"); + } + + return TermQueryBuilderProtoUtils.fromProto(queryContainer.getTerm()); + } +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java index 328ceea4a65c1..cc896d703cb0e 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoUtils.java @@ -27,6 +27,81 @@ private TermQueryBuilderProtoUtils() { // Utility class, no instances } + /** + * Converts a Protocol Buffer TermQuery to an OpenSearch TermQueryBuilder. + * Similar to {@link TermQueryBuilder#fromXContent(XContentParser)}, this method + * parses the Protocol Buffer representation and creates a properly configured + * TermQueryBuilder with the appropriate field name, value, boost, query name, + * and case sensitivity settings. + * + * @param termQueryProto The Protocol Buffer TermQuery object + * @return A configured TermQueryBuilder instance + * @throws IllegalArgumentException if the field value type is not supported, or if the term query field value is not recognized + */ + protected static TermQueryBuilder fromProto(TermQuery termQueryProto) { + String queryName = null; + String fieldName = termQueryProto.getField(); + Object value = null; + float boost = AbstractQueryBuilder.DEFAULT_BOOST; + boolean caseInsensitive = TermQueryBuilder.DEFAULT_CASE_INSENSITIVITY; + + if (termQueryProto.hasName()) { + queryName = termQueryProto.getName(); + } + if (termQueryProto.hasBoost()) { + boost = termQueryProto.getBoost(); + } + + FieldValue fieldValue = termQueryProto.getValue(); + + switch (fieldValue.getTypeCase()) { + case GENERAL_NUMBER: + switch (fieldValue.getGeneralNumber().getValueCase()) { + case INT32_VALUE: + value = fieldValue.getGeneralNumber().getInt32Value(); + break; + case INT64_VALUE: + value = fieldValue.getGeneralNumber().getInt64Value(); + break; + case FLOAT_VALUE: + value = fieldValue.getGeneralNumber().getFloatValue(); + break; + case DOUBLE_VALUE: + value = fieldValue.getGeneralNumber().getDoubleValue(); + break; + default: + throw new IllegalArgumentException( + "Unsupported general number type: " + fieldValue.getGeneralNumber().getValueCase() + ); + } + break; + case STRING_VALUE: + value = fieldValue.getStringValue(); + break; + case OBJECT_MAP: + value = ObjectMapProtoUtils.fromProto(fieldValue.getObjectMap()); + break; + case BOOL_VALUE: + value = fieldValue.getBoolValue(); + break; + default: + throw new IllegalArgumentException("TermQuery field value not recognized"); + } + + if (termQueryProto.hasCaseInsensitive()) { + caseInsensitive = termQueryProto.getCaseInsensitive(); + } + + TermQueryBuilder termQuery = new TermQueryBuilder(fieldName, value); + termQuery.boost(boost); + if (queryName != null) { + termQuery.queryName(queryName); + } + termQuery.caseInsensitive(caseInsensitive); + + return termQuery; + } + /** * Converts a Protocol Buffer TermQuery map to an OpenSearch TermQueryBuilder. * Similar to {@link TermQueryBuilder#fromXContent(XContentParser)}, this method @@ -38,7 +113,9 @@ private TermQueryBuilderProtoUtils() { * @return A configured TermQueryBuilder instance * @throws IllegalArgumentException if the term query map has more than one element, * if the field value type is not supported, or if the term query field value is not recognized + * @deprecated Use {@link #fromProto(TermQuery)} instead for the new protobuf structure */ + @Deprecated protected static TermQueryBuilder fromProto(Map termQueryProto) { String queryName = null; String fieldName = null; diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverter.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverter.java new file mode 100644 index 0000000000000..6a907d6852032 --- /dev/null +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverter.java @@ -0,0 +1,40 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.QueryContainer; + +/** + * Converter for Terms queries. + * This class implements the QueryBuilderProtoConverter interface to provide Terms query support + * for the gRPC transport plugin. + */ +public class TermsQueryBuilderProtoConverter implements QueryBuilderProtoConverter { + + /** + * Constructs a new TermsQueryBuilderProtoConverter. + */ + public TermsQueryBuilderProtoConverter() { + // Default constructor + } + + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.TERMS; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + if (queryContainer == null || !queryContainer.hasTerms()) { + throw new IllegalArgumentException("QueryContainer does not contain a Terms query"); + } + + return TermsQueryBuilderProtoUtils.fromProto(queryContainer.getTerms()); + } +} diff --git a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImpl.java b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImpl.java index 65f55d042ea20..342b5eef13ca8 100644 --- a/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImpl.java +++ b/plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImpl.java @@ -12,6 +12,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.plugin.transport.grpc.listeners.SearchRequestActionListener; import org.opensearch.plugin.transport.grpc.proto.request.search.SearchRequestProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; import org.opensearch.protobufs.services.SearchServiceGrpc; import org.opensearch.transport.client.Client; @@ -27,15 +28,24 @@ public class SearchServiceImpl extends SearchServiceGrpc.SearchServiceImplBase { private static final Logger logger = LogManager.getLogger(SearchServiceImpl.class); private final Client client; + private final AbstractQueryBuilderProtoUtils queryUtils; /** * Creates a new SearchServiceImpl. * - * @param client: Client for executing actions on the local node + * @param client Client for executing actions on the local node + * @param queryUtils Query utils instance for parsing protobuf queries */ - public SearchServiceImpl(Client client) { + public SearchServiceImpl(Client client, AbstractQueryBuilderProtoUtils queryUtils) { + if (client == null) { + throw new IllegalArgumentException("Client cannot be null"); + } + if (queryUtils == null) { + throw new IllegalArgumentException("Query utils cannot be null"); + } this.client = client; + this.queryUtils = queryUtils; } /** @@ -51,7 +61,7 @@ public void search( ) { try { - org.opensearch.action.search.SearchRequest searchRequest = SearchRequestProtoUtils.prepareRequest(request, client); + org.opensearch.action.search.SearchRequest searchRequest = SearchRequestProtoUtils.prepareRequest(request, client, queryUtils); SearchRequestActionListener listener = new SearchRequestActionListener(responseObserver); client.search(searchRequest, listener); } catch (RuntimeException | IOException e) { diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/GrpcPluginTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/GrpcPluginTests.java index 6b8d4b9f6596b..c19cecae8771b 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/GrpcPluginTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/GrpcPluginTests.java @@ -13,7 +13,10 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.core.indices.breaker.CircuitBreakerService; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter; import org.opensearch.plugin.transport.grpc.ssl.SecureNetty4GrpcServerTransport; +import org.opensearch.plugins.ExtensiblePlugin; +import org.opensearch.protobufs.QueryContainer; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -21,11 +24,14 @@ import org.opensearch.transport.client.Client; import org.junit.Before; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.function.Supplier; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import static org.opensearch.plugin.transport.grpc.Netty4GrpcServerTransport.GRPC_TRANSPORT_SETTING_KEY; @@ -42,6 +48,7 @@ import static org.opensearch.plugin.transport.grpc.ssl.SecureNetty4GrpcServerTransport.GRPC_SECURE_TRANSPORT_SETTING_KEY; import static org.opensearch.plugin.transport.grpc.ssl.SecureNetty4GrpcServerTransport.SETTING_GRPC_SECURE_PORT; import static org.opensearch.plugin.transport.grpc.ssl.SecureSettingsHelpers.getServerClientAuthNone; +import static org.mockito.Mockito.when; public class GrpcPluginTests extends OpenSearchTestCase { @@ -63,6 +70,9 @@ public class GrpcPluginTests extends OpenSearchTestCase { @Mock private Tracer tracer; + @Mock + private ExtensiblePlugin.ExtensionLoader extensionLoader; + @Before public void setup() { MockitoAnnotations.openMocks(this); @@ -113,6 +123,21 @@ public void testGetSettings() { assertEquals("Should return 11 settings", 11, settings.size()); } + public void testGetQueryUtilsBeforeCreateComponents() { + // Create a new plugin instance without calling createComponents + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Test that getQueryUtils throws IllegalStateException when queryUtils is not initialized + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> newPlugin.getQueryUtils()); + + assertEquals("Query utils not initialized. Make sure createComponents has been called.", exception.getMessage()); + } + + public void testGetQueryUtilsAfterCreateComponents() { + // Test that getQueryUtils returns the queryUtils instance after createComponents is called + assertNotNull("QueryUtils should not be null after createComponents", plugin.getQueryUtils()); + } + public void testGetAuxTransports() { Settings settings = Settings.builder().put(SETTING_GRPC_PORT.getKey(), "9200-9300").build(); @@ -153,4 +178,151 @@ public void testGetSecureAuxTransports() { AuxTransport transport = transports.get(GRPC_SECURE_TRANSPORT_SETTING_KEY).get(); assertTrue("Should return a SecureNetty4GrpcServerTransport instance", transport instanceof SecureNetty4GrpcServerTransport); } + + public void testGetAuxTransportsWithNullClient() { + Settings settings = Settings.builder().put(SETTING_GRPC_PORT.getKey(), "9200-9300").build(); + + // Create a new plugin instance without initializing the client + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Expect a RuntimeException when client is null + RuntimeException exception = expectThrows( + RuntimeException.class, + () -> newPlugin.getAuxTransports(settings, threadPool, circuitBreakerService, networkService, clusterSettings, tracer) + ); + + assertEquals("client cannot be null", exception.getMessage()); + } + + public void testGetSecureAuxTransportsWithNullClient() { + Settings settings = Settings.builder().put(SETTING_GRPC_SECURE_PORT.getKey(), "9200-9300").build(); + + // Create a new plugin instance without initializing the client + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Expect a RuntimeException when client is null + RuntimeException exception = expectThrows( + RuntimeException.class, + () -> newPlugin.getSecureAuxTransports( + settings, + threadPool, + circuitBreakerService, + networkService, + clusterSettings, + getServerClientAuthNone(), + tracer + ) + ); + + assertEquals("client cannot be null", exception.getMessage()); + } + + public void testLoadExtensions() { + // Create a new plugin instance + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Create a mock extension loader + ExtensiblePlugin.ExtensionLoader mockLoader = Mockito.mock(ExtensiblePlugin.ExtensionLoader.class); + + // Create a list of mock converters + List mockConverters = new ArrayList<>(); + mockConverters.add(Mockito.mock(QueryBuilderProtoConverter.class)); + mockConverters.add(Mockito.mock(QueryBuilderProtoConverter.class)); + + // Set up the mock loader to return the mock converters + when(mockLoader.loadExtensions(QueryBuilderProtoConverter.class)).thenReturn(mockConverters); + + // Call loadExtensions + newPlugin.loadExtensions(mockLoader); + + // Verify that the converters were loaded + assertEquals(2, newPlugin.getQueryConverters().size()); + } + + public void testLoadExtensionsWithNullExtensions() { + // Create a new plugin instance + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Create a mock extension loader + ExtensiblePlugin.ExtensionLoader mockLoader = Mockito.mock(ExtensiblePlugin.ExtensionLoader.class); + + // Set up the mock loader to return null + when(mockLoader.loadExtensions(QueryBuilderProtoConverter.class)).thenReturn(null); + + // Call loadExtensions + newPlugin.loadExtensions(mockLoader); + + // Verify that no converters were loaded + assertEquals(0, newPlugin.getQueryConverters().size()); + } + + public void testCreateComponents() { + // Create a new plugin instance + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Create a mock converter + QueryBuilderProtoConverter mockConverter = Mockito.mock(QueryBuilderProtoConverter.class); + + // Add the mock converter to the plugin + newPlugin.loadExtensions(extensionLoader); + when(extensionLoader.loadExtensions(QueryBuilderProtoConverter.class)).thenReturn(List.of(mockConverter)); + + // Call createComponents + Collection components = newPlugin.createComponents( + client, + null, // ClusterService + null, // ThreadPool + null, // ResourceWatcherService + null, // ScriptService + null, // NamedXContentRegistry + null, // Environment + null, // NodeEnvironment + null, // NamedWriteableRegistry + null, // IndexNameExpressionResolver + null // Supplier + ); + + // Verify that the queryUtils instance was created and is available + assertNotNull("QueryUtils should be initialized after createComponents", newPlugin.getQueryUtils()); + } + + public void testCreateComponentsWithExternalConverters() { + // Create a new plugin instance + GrpcPlugin newPlugin = new GrpcPlugin(); + + // Create a mock converter that will be registered + QueryBuilderProtoConverter mockConverter = Mockito.mock(QueryBuilderProtoConverter.class); + when(mockConverter.getHandledQueryCase()).thenReturn(QueryContainer.QueryContainerCase.MATCH_ALL); + + // Create a mock extension loader that returns the converter + ExtensiblePlugin.ExtensionLoader mockLoader = Mockito.mock(ExtensiblePlugin.ExtensionLoader.class); + when(mockLoader.loadExtensions(QueryBuilderProtoConverter.class)).thenReturn(List.of(mockConverter)); + + // Load the extensions first + newPlugin.loadExtensions(mockLoader); + + // Verify the converter was added to the queryConverters list + assertEquals("Should have 1 query converter loaded", 1, newPlugin.getQueryConverters().size()); + + // Call createComponents to trigger registration of external converters + Collection components = newPlugin.createComponents( + client, + null, // ClusterService + null, // ThreadPool + null, // ResourceWatcherService + null, // ScriptService + null, // NamedXContentRegistry + null, // Environment + null, // NodeEnvironment + null, // NamedWriteableRegistry + null, // IndexNameExpressionResolver + null // Supplier + ); + + // Verify that the queryUtils instance was created and is available + assertNotNull("QueryUtils should be initialized after createComponents", newPlugin.getQueryUtils()); + + // Verify that the external converter was registered by checking it was called + Mockito.verify(mockConverter).getHandledQueryCase(); + } } diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java index 87b46c20fa391..97cd9674b6317 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java @@ -13,6 +13,8 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoTestUtils; import org.opensearch.protobufs.SearchRequestBody; import org.opensearch.protobufs.SourceConfigParam; import org.opensearch.protobufs.TrackHits; @@ -27,19 +29,20 @@ import java.io.IOException; -import static org.opensearch.search.internal.SearchContext.TRACK_TOTAL_HITS_DISABLED; import static org.mockito.Mockito.mock; public class SearchRequestProtoUtilsTests extends OpenSearchTestCase { private NamedWriteableRegistry namedWriteableRegistry; private Client mockClient; + private AbstractQueryBuilderProtoUtils queryUtils; @Override public void setUp() throws Exception { super.setUp(); namedWriteableRegistry = mock(NamedWriteableRegistry.class); mockClient = mock(Client.class); + queryUtils = QueryBuilderProtoTestUtils.createQueryUtils(); } public void testParseSearchRequestWithBasicFields() throws IOException { @@ -67,7 +70,7 @@ public void testParseSearchRequestWithBasicFields() throws IOException { SearchRequest searchRequest = new SearchRequest(); // Call the method under test - SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}); + SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils); // Verify the result assertNotNull("SearchRequest should not be null", searchRequest); @@ -115,7 +118,7 @@ public void testParseSearchRequestWithRequestBody() throws IOException { SearchRequest searchRequest = new SearchRequest(); // Call the method under test - SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}); + SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils); // Verify the result assertNotNull("SearchRequest should not be null", searchRequest); @@ -330,7 +333,7 @@ public void testCheckProtoTotalHitsWithRestTotalHitsAsInt() throws IOException { // Verify the result assertNotNull("SearchRequest should not be null", searchRequest); assertNotNull("Source should not be null", searchRequest.source()); - assertTrue("TrackTotalHits should be true", searchRequest.source().trackTotalHitsUpTo().intValue() > TRACK_TOTAL_HITS_DISABLED); + assertTrue("TrackTotalHits should be true", searchRequest.source().trackTotalHitsUpTo() == SearchContext.TRACK_TOTAL_HITS_ACCURATE); } public void testCheckProtoTotalHitsWithTrackTotalHitsUpTo() throws IOException { diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java index 93be2a2442a52..3460b58419659 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java @@ -11,6 +11,8 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.index.query.MatchAllQueryBuilder; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoTestUtils; import org.opensearch.protobufs.DerivedField; import org.opensearch.protobufs.FieldAndFormat; import org.opensearch.protobufs.FieldValue; @@ -40,11 +42,14 @@ public class SearchSourceBuilderProtoUtilsTests extends OpenSearchTestCase { private NamedWriteableRegistry mockRegistry; + private AbstractQueryBuilderProtoUtils queryUtils; @Override public void setUp() throws Exception { super.setUp(); mockRegistry = mock(NamedWriteableRegistry.class); + // Create an instance with all built-in converters + queryUtils = QueryBuilderProtoTestUtils.createQueryUtils(); } public void testParseProtoWithFrom() throws IOException { @@ -55,7 +60,7 @@ public void testParseProtoWithFrom() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("From should match", 10, searchSourceBuilder.from()); @@ -69,7 +74,7 @@ public void testParseProtoWithSize() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("Size should match", 20, searchSourceBuilder.size()); @@ -83,7 +88,7 @@ public void testParseProtoWithTimeout() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("Timeout should match", TimeValue.timeValueSeconds(5), searchSourceBuilder.timeout()); @@ -97,7 +102,7 @@ public void testParseProtoWithTerminateAfter() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("TerminateAfter should match", 100, searchSourceBuilder.terminateAfter()); @@ -111,7 +116,7 @@ public void testParseProtoWithMinScore() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("MinScore should match", 0.5f, searchSourceBuilder.minScore(), 0.0f); @@ -125,7 +130,7 @@ public void testParseProtoWithVersion() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("Version should be true", searchSourceBuilder.version()); @@ -139,7 +144,7 @@ public void testParseProtoWithSeqNoPrimaryTerm() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("SeqNoPrimaryTerm should be true", searchSourceBuilder.seqNoAndPrimaryTerm()); @@ -153,7 +158,7 @@ public void testParseProtoWithExplain() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("Explain should be true", searchSourceBuilder.explain()); @@ -167,7 +172,7 @@ public void testParseProtoWithTrackScores() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("TrackScores should be true", searchSourceBuilder.trackScores()); @@ -181,7 +186,7 @@ public void testParseProtoWithIncludeNamedQueriesScore() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result searchSourceBuilder.includeNamedQueriesScores(true); @@ -198,7 +203,7 @@ public void testParseProtoWithTrackTotalHitsBooleanTrue() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("TrackTotalHits should be accurate", TRACK_TOTAL_HITS_ACCURATE, searchSourceBuilder.trackTotalHitsUpTo().intValue()); @@ -214,7 +219,7 @@ public void testParseProtoWithTrackTotalHitsBooleanFalse() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("TrackTotalHits should be disabled", TRACK_TOTAL_HITS_DISABLED, searchSourceBuilder.trackTotalHitsUpTo().intValue()); @@ -230,7 +235,7 @@ public void testParseProtoWithTrackTotalHitsInteger() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("TrackTotalHits should match", 1000, searchSourceBuilder.trackTotalHitsUpTo().intValue()); @@ -244,7 +249,7 @@ public void testParseProtoWithProfile() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("Profile should be true", searchSourceBuilder.profile()); @@ -258,7 +263,7 @@ public void testParseProtoWithSearchPipeline() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertEquals("SearchPipeline should match", "my-pipeline", searchSourceBuilder.pipeline()); @@ -272,7 +277,7 @@ public void testParseProtoWithVerbosePipeline() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertTrue("VerbosePipeline should be true", searchSourceBuilder.verbosePipeline()); @@ -288,7 +293,7 @@ public void testParseProtoWithQuery() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("Query should not be null", searchSourceBuilder.query()); @@ -303,7 +308,7 @@ public void testParseProtoWithStats() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("Stats should not be null", searchSourceBuilder.stats()); @@ -323,7 +328,7 @@ public void testParseProtoWithDocValueFields() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("DocValueFields should not be null", searchSourceBuilder.docValueFields()); @@ -341,7 +346,7 @@ public void testParseProtoWithFields() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("FetchFields should not be null", searchSourceBuilder.fetchFields()); @@ -362,7 +367,7 @@ public void testParseProtoWithIndicesBoost() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("IndexBoosts should not be null", searchSourceBuilder.indexBoosts()); @@ -379,7 +384,7 @@ public void testParseProtoWithSortString() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("Sorts should not be null", searchSourceBuilder.sorts()); @@ -396,7 +401,7 @@ public void testParseProtoWithPostFilter() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("PostFilter should not be null", searchSourceBuilder.postFilter()); @@ -430,7 +435,7 @@ public void testParseProtoWithScriptFields() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("ScriptFields should not be null", searchSourceBuilder.scriptFields()); @@ -461,7 +466,7 @@ public void testParseProtoWithSlice() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("Slice should not be null", searchSourceBuilder.slice()); @@ -498,7 +503,7 @@ public void testParseProtoWithDerivedFields() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("DerivedFields should not be null", searchSourceBuilder.getDerivedFields()); @@ -538,7 +543,7 @@ public void testParseProtoWithSearchAfter() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest); + SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); // Verify the result assertNotNull("SearchAfter should not be null", searchSourceBuilder.searchAfter()); @@ -557,7 +562,7 @@ public void testParseProtoWithExtThrowsUnsupportedOperationException() throws IO // Call the method under test, should throw UnsupportedOperationException UnsupportedOperationException exception = expectThrows( UnsupportedOperationException.class, - () -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest) + () -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils) ); assertTrue("Exception message should mention ext param", exception.getMessage().contains("ext param is not supported yet")); diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtilsTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtilsTests.java index 0419f3f4976bb..f7a9b10adbf35 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtilsTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/AbstractQueryBuilderProtoUtilsTests.java @@ -28,13 +28,39 @@ public class AbstractQueryBuilderProtoUtilsTests extends OpenSearchTestCase { + private AbstractQueryBuilderProtoUtils queryUtils; + + @Override + public void setUp() throws Exception { + super.setUp(); + // Create an instance with all built-in converters + queryUtils = QueryBuilderProtoTestUtils.createQueryUtils(); + } + + public void testConstructorWithNullRegistry() { + // Test that constructor throws IllegalArgumentException when registry is null + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new AbstractQueryBuilderProtoUtils(null)); + + assertEquals("Registry cannot be null", exception.getMessage()); + } + + public void testParseInnerQueryBuilderProtoWithNullContainer() { + // Test that method throws IllegalArgumentException when queryContainer is null + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> queryUtils.parseInnerQueryBuilderProto(null) + ); + + assertEquals("Query container cannot be null", exception.getMessage()); + } + public void testParseInnerQueryBuilderProtoWithMatchAll() { // Create a QueryContainer with MatchAllQuery MatchAllQuery matchAllQuery = MatchAllQuery.newBuilder().build(); QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(matchAllQuery).build(); - // Call parseInnerQueryBuilderProto - QueryBuilder queryBuilder = AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(queryContainer); + // Call parseInnerQueryBuilderProto using instance method + QueryBuilder queryBuilder = queryUtils.parseInnerQueryBuilderProto(queryContainer); // Verify the result assertNotNull("QueryBuilder should not be null", queryBuilder); @@ -46,8 +72,8 @@ public void testParseInnerQueryBuilderProtoWithMatchNone() { MatchNoneQuery matchNoneQuery = MatchNoneQuery.newBuilder().build(); QueryContainer queryContainer = QueryContainer.newBuilder().setMatchNone(matchNoneQuery).build(); - // Call parseInnerQueryBuilderProto - QueryBuilder queryBuilder = AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(queryContainer); + // Call parseInnerQueryBuilderProto using instance method + QueryBuilder queryBuilder = queryUtils.parseInnerQueryBuilderProto(queryContainer); // Verify the result assertNotNull("QueryBuilder should not be null", queryBuilder); @@ -56,20 +82,13 @@ public void testParseInnerQueryBuilderProtoWithMatchNone() { public void testParseInnerQueryBuilderProtoWithTerm() { // Create a QueryContainer with Term query - Map termMap = new HashMap<>(); - - // Create a FieldValue for the term value FieldValue fieldValue = FieldValue.newBuilder().setStringValue("test-value").build(); + TermQuery termQuery = TermQuery.newBuilder().setField("test-field").setValue(fieldValue).build(); - // Create a TermQuery with the FieldValue - TermQuery termQuery = TermQuery.newBuilder().setValue(fieldValue).build(); - - termMap.put("test-field", termQuery); - - QueryContainer queryContainer = QueryContainer.newBuilder().putAllTerm(termMap).build(); + QueryContainer queryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); - // Call parseInnerQueryBuilderProto - QueryBuilder queryBuilder = AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(queryContainer); + // Call parseInnerQueryBuilderProto using instance method + QueryBuilder queryBuilder = queryUtils.parseInnerQueryBuilderProto(queryContainer); // Verify the result assertNotNull("QueryBuilder should not be null", queryBuilder); @@ -80,19 +99,16 @@ public void testParseInnerQueryBuilderProtoWithTerm() { } public void testParseInnerQueryBuilderProtoWithTerms() { - // Create a StringArray for terms values + // Create a QueryContainer with Terms query using the correct protobuf classes StringArray stringArray = StringArray.newBuilder().addStringArray("value1").addStringArray("value2").build(); - // Create a TermsLookupFieldStringArrayMap TermsLookupFieldStringArrayMap termsLookupFieldStringArrayMap = TermsLookupFieldStringArrayMap.newBuilder() .setStringArray(stringArray) .build(); - // Create a map for TermsLookupFieldStringArrayMap Map termsLookupFieldStringArrayMapMap = new HashMap<>(); termsLookupFieldStringArrayMapMap.put("test-field", termsLookupFieldStringArrayMap); - // Create a TermsQueryField TermsQueryField termsQueryField = TermsQueryField.newBuilder() .putAllTermsLookupFieldStringArrayMap(termsLookupFieldStringArrayMapMap) .build(); @@ -100,8 +116,8 @@ public void testParseInnerQueryBuilderProtoWithTerms() { // Create a QueryContainer with Terms query QueryContainer queryContainer = QueryContainer.newBuilder().setTerms(termsQueryField).build(); - // Call parseInnerQueryBuilderProto - QueryBuilder queryBuilder = AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(queryContainer); + // Call parseInnerQueryBuilderProto using instance method + QueryBuilder queryBuilder = queryUtils.parseInnerQueryBuilderProto(queryContainer); // Verify the result assertNotNull("QueryBuilder should not be null", queryBuilder); @@ -113,17 +129,17 @@ public void testParseInnerQueryBuilderProtoWithTerms() { assertEquals("Second value should match", "value2", termsQueryBuilder.values().get(1)); } - public void testParseInnerQueryBuilderProtoWithUnsupportedQueryType() { + public void testParseInnerQueryBuilderProtoWithUnsupportedQuery() { // Create an empty QueryContainer (no query type specified) QueryContainer queryContainer = QueryContainer.newBuilder().build(); - // Call parseInnerQueryBuilderProto, should throw UnsupportedOperationException - UnsupportedOperationException exception = expectThrows( - UnsupportedOperationException.class, - () -> AbstractQueryBuilderProtoUtils.parseInnerQueryBuilderProto(queryContainer) + // Call parseInnerQueryBuilderProto using instance method, should throw IllegalArgumentException + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> queryUtils.parseInnerQueryBuilderProto(queryContainer) ); // Verify the exception message - assertTrue("Exception message should mention 'not supported yet'", exception.getMessage().contains("not supported yet")); + assertTrue("Exception message should mention 'Unsupported query type'", exception.getMessage().contains("Unsupported query type")); } } diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/EmptyQueryBuilderProtoConverterRegistry.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/EmptyQueryBuilderProtoConverterRegistry.java new file mode 100644 index 0000000000000..10fa60e7ae4e6 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/EmptyQueryBuilderProtoConverterRegistry.java @@ -0,0 +1,49 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * A test-specific implementation of QueryBuilderProtoConverterRegistry that starts with no converters. + * This class is used for testing scenarios where we need a clean registry without any built-in converters. + */ +public class EmptyQueryBuilderProtoConverterRegistry extends QueryBuilderProtoConverterRegistry { + + private static final Logger logger = LogManager.getLogger(EmptyQueryBuilderProtoConverterRegistry.class); + + /** + * Creates a new empty registry with no converters. + * This constructor calls the parent constructor but doesn't register any converters. + */ + public EmptyQueryBuilderProtoConverterRegistry() { + // The parent constructor will call registerBuiltInConverters() and loadExternalConverters(), + // but we'll override those methods to do nothing + } + + /** + * Override the parent's registerBuiltInConverters method to do nothing. + * This ensures no built-in converters are registered. + */ + @Override + protected void registerBuiltInConverters() { + // Do nothing - we want an empty registry for testing + logger.debug("Skipping registration of built-in converters for testing"); + } + + /** + * Override the parent's loadExternalConverters method to do nothing. + * This ensures no external converters are loaded. + */ + @Override + protected void loadExternalConverters() { + // Do nothing - we want an empty registry for testing + logger.debug("Skipping loading of external converters for testing"); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000000..ab0e106e48b79 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchAllQueryBuilderProtoConverterTests.java @@ -0,0 +1,64 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.MatchAllQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.MatchAllQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.test.OpenSearchTestCase; + +public class MatchAllQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private MatchAllQueryBuilderProtoConverter converter; + + @Override + public void setUp() throws Exception { + super.setUp(); + converter = new MatchAllQueryBuilderProtoConverter(); + } + + public void testGetHandledQueryCase() { + // Test that the converter returns the correct QueryContainerCase + assertEquals( + "Converter should handle MATCH_ALL case", + QueryContainer.QueryContainerCase.MATCH_ALL, + converter.getHandledQueryCase() + ); + } + + public void testFromProto() { + // Create a QueryContainer with MatchAllQuery + MatchAllQuery matchAllQuery = MatchAllQuery.newBuilder().setBoost(2.0f).setName("test_query").build(); + QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(matchAllQuery).build(); + + // Convert the query + QueryBuilder queryBuilder = converter.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertTrue("QueryBuilder should be a MatchAllQueryBuilder", queryBuilder instanceof MatchAllQueryBuilder); + MatchAllQueryBuilder matchAllQueryBuilder = (MatchAllQueryBuilder) queryBuilder; + assertEquals("Boost should match", 2.0f, matchAllQueryBuilder.boost(), 0.0f); + assertEquals("Query name should match", "test_query", matchAllQueryBuilder.queryName()); + } + + public void testFromProtoWithInvalidContainer() { + // Create a QueryContainer with a different query type + QueryContainer emptyContainer = QueryContainer.newBuilder().build(); + + // Test that the converter throws an exception + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(emptyContainer)); + + // Verify the exception message + assertTrue( + "Exception message should mention 'does not contain a MatchAll query'", + exception.getMessage().contains("does not contain a MatchAll query") + ); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000000..92ab02cc795f7 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/MatchNoneQueryBuilderProtoConverterTests.java @@ -0,0 +1,64 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.MatchNoneQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.MatchNoneQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.test.OpenSearchTestCase; + +public class MatchNoneQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private MatchNoneQueryBuilderProtoConverter converter; + + @Override + public void setUp() throws Exception { + super.setUp(); + converter = new MatchNoneQueryBuilderProtoConverter(); + } + + public void testGetHandledQueryCase() { + // Test that the converter returns the correct QueryContainerCase + assertEquals( + "Converter should handle MATCH_NONE case", + QueryContainer.QueryContainerCase.MATCH_NONE, + converter.getHandledQueryCase() + ); + } + + public void testFromProto() { + // Create a QueryContainer with MatchNoneQuery + MatchNoneQuery matchNoneQuery = MatchNoneQuery.newBuilder().setBoost(2.0f).setName("test_query").build(); + QueryContainer queryContainer = QueryContainer.newBuilder().setMatchNone(matchNoneQuery).build(); + + // Convert the query + QueryBuilder queryBuilder = converter.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertTrue("QueryBuilder should be a MatchNoneQueryBuilder", queryBuilder instanceof MatchNoneQueryBuilder); + MatchNoneQueryBuilder matchNoneQueryBuilder = (MatchNoneQueryBuilder) queryBuilder; + assertEquals("Boost should match", 2.0f, matchNoneQueryBuilder.boost(), 0.0f); + assertEquals("Query name should match", "test_query", matchNoneQueryBuilder.queryName()); + } + + public void testFromProtoWithInvalidContainer() { + // Create a QueryContainer with a different query type + QueryContainer emptyContainer = QueryContainer.newBuilder().build(); + + // Test that the converter throws an exception + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(emptyContainer)); + + // Verify the exception message + assertTrue( + "Exception message should mention 'does not contain a MatchNone query'", + exception.getMessage().contains("does not contain a MatchNone query") + ); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java new file mode 100644 index 0000000000000..04b92de0a0504 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java @@ -0,0 +1,134 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.FieldValue; +import org.opensearch.protobufs.MatchAllQuery; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.protobufs.TermQuery; +import org.opensearch.test.OpenSearchTestCase; + +/** + * Test class for QueryBuilderProtoConverterRegistry to verify the map-based optimization. + */ +public class QueryBuilderProtoConverterRegistryTests extends OpenSearchTestCase { + + private QueryBuilderProtoConverterRegistry registry; + + @Override + public void setUp() throws Exception { + super.setUp(); + registry = new QueryBuilderProtoConverterRegistry(); + } + + public void testMatchAllQueryConversion() { + // Create a MatchAll query container + QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(MatchAllQuery.newBuilder().build()).build(); + + // Convert using the registry + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals( + "Should be a MatchAllQueryBuilder", + "org.opensearch.index.query.MatchAllQueryBuilder", + queryBuilder.getClass().getName() + ); + } + + public void testTermQueryConversion() { + // Create a Term query container + QueryContainer queryContainer = QueryContainer.newBuilder() + .setTerm( + TermQuery.newBuilder().setField("test_field").setValue(FieldValue.newBuilder().setStringValue("test_value").build()).build() + ) + .build(); + + // Convert using the registry + QueryBuilder queryBuilder = registry.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertEquals("Should be a TermQueryBuilder", "org.opensearch.index.query.TermQueryBuilder", queryBuilder.getClass().getName()); + } + + public void testNullQueryContainer() { + expectThrows(IllegalArgumentException.class, () -> registry.fromProto(null)); + } + + public void testUnsupportedQueryType() { + // Create an empty query container (no query type set) + QueryContainer queryContainer = QueryContainer.newBuilder().build(); + expectThrows(IllegalArgumentException.class, () -> registry.fromProto(queryContainer)); + } + + public void testConverterRegistration() { + // Create a custom converter for testing + QueryBuilderProtoConverter customConverter = new QueryBuilderProtoConverter() { + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.MATCH_ALL; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + // Return a mock QueryBuilder for testing + return new org.opensearch.index.query.MatchAllQueryBuilder(); + } + }; + + // Register the custom converter + registry.registerConverter(customConverter); + + // Test that it works + QueryContainer queryContainer = QueryContainer.newBuilder().setMatchAll(MatchAllQuery.newBuilder().build()).build(); + + QueryBuilder result = registry.fromProto(queryContainer); + assertNotNull("Result should not be null", result); + } + + public void testNullConverter() { + expectThrows(IllegalArgumentException.class, () -> registry.registerConverter(null)); + } + + public void testNullHandledQueryCase() { + // Create a custom converter that returns null for getHandledQueryCase + QueryBuilderProtoConverter customConverter = new QueryBuilderProtoConverter() { + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return null; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + return new org.opensearch.index.query.MatchAllQueryBuilder(); + } + }; + + expectThrows(IllegalArgumentException.class, () -> registry.registerConverter(customConverter)); + } + + public void testNotSetHandledQueryCase() { + // Create a custom converter that returns QUERYCONTAINER_NOT_SET for getHandledQueryCase + QueryBuilderProtoConverter customConverter = new QueryBuilderProtoConverter() { + @Override + public QueryContainer.QueryContainerCase getHandledQueryCase() { + return QueryContainer.QueryContainerCase.QUERYCONTAINER_NOT_SET; + } + + @Override + public QueryBuilder fromProto(QueryContainer queryContainer) { + return new org.opensearch.index.query.MatchAllQueryBuilder(); + } + }; + + expectThrows(IllegalArgumentException.class, () -> registry.registerConverter(customConverter)); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoTestUtils.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoTestUtils.java new file mode 100644 index 0000000000000..d8c3a2edf363b --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/QueryBuilderProtoTestUtils.java @@ -0,0 +1,51 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +/** + * Utility class for setting up query builder proto converters in tests. + */ +public class QueryBuilderProtoTestUtils { + + private QueryBuilderProtoTestUtils() { + // Utility class, no instances + } + + /** + * Creates an AbstractQueryBuilderProtoUtils instance with all built-in converters for testing. + * This method should be called in test methods that need query parsing functionality. + * + * @return AbstractQueryBuilderProtoUtils instance configured for testing + */ + public static AbstractQueryBuilderProtoUtils createQueryUtils() { + // Create a new registry + QueryBuilderProtoConverterRegistry registry = new QueryBuilderProtoConverterRegistry(); + + // Register all built-in converters + registry.registerConverter(new MatchAllQueryBuilderProtoConverter()); + registry.registerConverter(new MatchNoneQueryBuilderProtoConverter()); + registry.registerConverter(new TermQueryBuilderProtoConverter()); + registry.registerConverter(new TermsQueryBuilderProtoConverter()); + + // Return an instance with the configured registry + return new AbstractQueryBuilderProtoUtils(registry); + } + + /** + * Sets up the registry with all built-in converters for testing. + * This method should be called in the setUp method of test classes + * that use AbstractQueryBuilderProtoUtils. + * + * @deprecated Use createQueryUtils() for instance-based approach instead + */ + @Deprecated + public static void setupRegistry() { + // This method is no longer used in the instance-based approach + // Kept for any remaining legacy test compatibility + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000000..ef32bdb44e0f3 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermQueryBuilderProtoConverterTests.java @@ -0,0 +1,71 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.TermQueryBuilder; +import org.opensearch.protobufs.FieldValue; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.protobufs.TermQuery; +import org.opensearch.test.OpenSearchTestCase; + +public class TermQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private TermQueryBuilderProtoConverter converter; + + @Override + public void setUp() throws Exception { + super.setUp(); + converter = new TermQueryBuilderProtoConverter(); + } + + public void testGetHandledQueryCase() { + // Test that the converter returns the correct QueryContainerCase + assertEquals("Converter should handle TERM case", QueryContainer.QueryContainerCase.TERM, converter.getHandledQueryCase()); + } + + public void testFromProto() { + // Create a QueryContainer with TermQuery + FieldValue fieldValue = FieldValue.newBuilder().setStringValue("test-value").build(); + TermQuery termQuery = TermQuery.newBuilder() + .setField("test-field") + .setValue(fieldValue) + .setBoost(2.0f) + .setName("test_query") + .setCaseInsensitive(true) + .build(); + QueryContainer queryContainer = QueryContainer.newBuilder().setTerm(termQuery).build(); + + // Convert the query + QueryBuilder queryBuilder = converter.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertTrue("QueryBuilder should be a TermQueryBuilder", queryBuilder instanceof TermQueryBuilder); + TermQueryBuilder termQueryBuilder = (TermQueryBuilder) queryBuilder; + assertEquals("Field name should match", "test-field", termQueryBuilder.fieldName()); + assertEquals("Value should match", "test-value", termQueryBuilder.value()); + assertEquals("Boost should match", 2.0f, termQueryBuilder.boost(), 0.0f); + assertEquals("Query name should match", "test_query", termQueryBuilder.queryName()); + assertTrue("Case insensitive should be true", termQueryBuilder.caseInsensitive()); + } + + public void testFromProtoWithInvalidContainer() { + // Create a QueryContainer with a different query type + QueryContainer emptyContainer = QueryContainer.newBuilder().build(); + + // Test that the converter throws an exception + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(emptyContainer)); + + // Verify the exception message + assertTrue( + "Exception message should mention 'does not contain a Term query'", + exception.getMessage().contains("does not contain a Term query") + ); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java new file mode 100644 index 0000000000000..99659c8ad28f2 --- /dev/null +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java @@ -0,0 +1,82 @@ +/* + * 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.plugin.transport.grpc.proto.request.search.query; + +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.TermsQueryBuilder; +import org.opensearch.protobufs.QueryContainer; +import org.opensearch.protobufs.StringArray; +import org.opensearch.protobufs.TermsLookupFieldStringArrayMap; +import org.opensearch.protobufs.TermsQueryField; +import org.opensearch.protobufs.ValueType; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.HashMap; +import java.util.Map; + +public class TermsQueryBuilderProtoConverterTests extends OpenSearchTestCase { + + private TermsQueryBuilderProtoConverter converter; + + @Override + public void setUp() throws Exception { + super.setUp(); + converter = new TermsQueryBuilderProtoConverter(); + } + + public void testGetHandledQueryCase() { + // Test that the converter returns the correct QueryContainerCase + assertEquals("Converter should handle TERMS case", QueryContainer.QueryContainerCase.TERMS, converter.getHandledQueryCase()); + } + + public void testFromProto() { + // Create a QueryContainer with TermsQuery + StringArray stringArray = StringArray.newBuilder().addStringArray("value1").addStringArray("value2").build(); + TermsLookupFieldStringArrayMap termsLookupFieldStringArrayMap = TermsLookupFieldStringArrayMap.newBuilder() + .setStringArray(stringArray) + .build(); + Map termsLookupFieldStringArrayMapMap = new HashMap<>(); + termsLookupFieldStringArrayMapMap.put("test-field", termsLookupFieldStringArrayMap); + TermsQueryField termsQueryField = TermsQueryField.newBuilder() + .putAllTermsLookupFieldStringArrayMap(termsLookupFieldStringArrayMapMap) + .setBoost(2.0f) + .setName("test_query") + .setValueType(ValueType.VALUE_TYPE_DEFAULT) + .build(); + QueryContainer queryContainer = QueryContainer.newBuilder().setTerms(termsQueryField).build(); + + // Convert the query + QueryBuilder queryBuilder = converter.fromProto(queryContainer); + + // Verify the result + assertNotNull("QueryBuilder should not be null", queryBuilder); + assertTrue("QueryBuilder should be a TermsQueryBuilder", queryBuilder instanceof TermsQueryBuilder); + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder; + assertEquals("Field name should match", "test-field", termsQueryBuilder.fieldName()); + assertEquals("Values size should match", 2, termsQueryBuilder.values().size()); + assertEquals("First value should match", "value1", termsQueryBuilder.values().get(0)); + assertEquals("Second value should match", "value2", termsQueryBuilder.values().get(1)); + assertEquals("Boost should match", 2.0f, termsQueryBuilder.boost(), 0.0f); + assertEquals("Query name should match", "test_query", termsQueryBuilder.queryName()); + assertEquals("Value type should match", TermsQueryBuilder.ValueType.DEFAULT, termsQueryBuilder.valueType()); + } + + public void testFromProtoWithInvalidContainer() { + // Create a QueryContainer with a different query type + QueryContainer emptyContainer = QueryContainer.newBuilder().build(); + + // Test that the converter throws an exception + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(emptyContainer)); + + // Verify the exception message + assertTrue( + "Exception message should mention 'does not contain a Terms query'", + exception.getMessage().contains("does not contain a Terms query") + ); + } +} diff --git a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImplTests.java b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImplTests.java index fddb00da495ce..a254d14e7c4a0 100644 --- a/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImplTests.java +++ b/plugins/transport-grpc/src/test/java/org/opensearch/plugin/transport/grpc/services/SearchServiceImplTests.java @@ -8,6 +8,8 @@ package org.opensearch.plugin.transport.grpc.services; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; +import org.opensearch.plugin.transport.grpc.proto.request.search.query.QueryBuilderProtoTestUtils; import org.opensearch.protobufs.SearchRequest; import org.opensearch.protobufs.SearchRequestBody; import org.opensearch.test.OpenSearchTestCase; @@ -27,6 +29,7 @@ public class SearchServiceImplTests extends OpenSearchTestCase { private SearchServiceImpl service; + private AbstractQueryBuilderProtoUtils queryUtils; @Mock private NodeClient client; @@ -37,7 +40,30 @@ public class SearchServiceImplTests extends OpenSearchTestCase { @Before public void setup() throws IOException { MockitoAnnotations.openMocks(this); - service = new SearchServiceImpl(client); + queryUtils = QueryBuilderProtoTestUtils.createQueryUtils(); + service = new SearchServiceImpl(client, queryUtils); + } + + public void testConstructorWithNullClient() { + // Test that constructor throws IllegalArgumentException when client is null + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new SearchServiceImpl(null, queryUtils)); + + assertEquals("Client cannot be null", exception.getMessage()); + } + + public void testConstructorWithNullQueryUtils() { + // Test that constructor throws IllegalArgumentException when queryUtils is null + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new SearchServiceImpl(client, null)); + + assertEquals("Query utils cannot be null", exception.getMessage()); + } + + public void testConstructorWithBothNull() { + // Test that constructor throws IllegalArgumentException when both parameters are null + // Should fail on the first null check (client) + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new SearchServiceImpl(null, null)); + + assertEquals("Client cannot be null", exception.getMessage()); } public void testSearchSuccess() throws IOException { @@ -51,23 +77,21 @@ public void testSearchSuccess() throws IOException { verify(client).search(any(org.opensearch.action.search.SearchRequest.class), any()); } - public void testSearchError() throws IOException { + public void testSearchWithException() throws IOException { // Create a test request SearchRequest request = createTestSearchRequest(); - // Make the client throw an exception when search is called - doThrow(new RuntimeException("Test exception")).when(client).search(any(org.opensearch.action.search.SearchRequest.class), any()); + // Mock client to throw an exception + doThrow(new RuntimeException("Test exception")).when(client).search(any(), any()); - // Call the search method + // Call search method service.search(request, responseObserver); - // Verify that the error was sent - verify(responseObserver).onError(any(RuntimeException.class)); + // Verify that responseObserver.onError was called + verify(responseObserver).onError(any()); } private SearchRequest createTestSearchRequest() { - SearchRequestBody requestBody = SearchRequestBody.newBuilder().build(); - - return SearchRequest.newBuilder().setRequestBody(requestBody).build(); + return SearchRequest.newBuilder().addIndex("test-index").setRequestBody(SearchRequestBody.newBuilder().setSize(10).build()).build(); } }