Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add skip_list param for date, scaled float and token count fields ([#19142](https://github.com/opensearch-project/OpenSearch/pull/19142))
- Implement GRPC MatchPhrase, MultiMatch queries ([#19449](https://github.com/opensearch-project/OpenSearch/pull/19449))
- Optimize gRPC transport thread management for improved throughput ([#19278](https://github.com/opensearch-project/OpenSearch/pull/19278))
- Implement GRPC Boolean query and inject registry for all internal query converters ([#19391](https://github.com/opensearch-project/OpenSearch/pull/19391))

### Changed
- Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ public Collection<Object> createComponents(
queryRegistry.registerConverter(converter);
}
logger.info("Successfully injected registry and registered all {} external converters", queryConverters.size());

// Update the registry on all converters (including built-in ones) so they can access external converters
queryRegistry.updateRegistryOnAllConverters();
logger.info("Updated registry on all converters to include external converters");
} else {
logger.info("No external QueryBuilderProtoConverter(s) to register");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/
package org.opensearch.transport.grpc.proto.request.search.query;

import org.opensearch.index.query.QueryBuilder;
import org.opensearch.protobufs.QueryContainer;
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverter;
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry;

/**
* Converter for Bool queries.
* This class implements the QueryBuilderProtoConverter interface to provide Bool query support
* for the gRPC transport module.
*/
public class BoolQueryBuilderProtoConverter implements QueryBuilderProtoConverter {

/**
* Default constructor for BoolQueryBuilderProtoConverter.
*/
public BoolQueryBuilderProtoConverter() {}

private QueryBuilderProtoConverterRegistry registry;

@Override
public void setRegistry(QueryBuilderProtoConverterRegistry registry) {
this.registry = registry;
}

@Override
public QueryContainer.QueryContainerCase getHandledQueryCase() {
return QueryContainer.QueryContainerCase.BOOL;
}

@Override
public QueryBuilder fromProto(QueryContainer queryContainer) {
if (queryContainer == null || queryContainer.getQueryContainerCase() != QueryContainer.QueryContainerCase.BOOL) {
throw new IllegalArgumentException("QueryContainer does not contain a Bool query");
}

return BoolQueryBuilderProtoUtils.fromProto(queryContainer.getBool(), registry);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/
package org.opensearch.transport.grpc.proto.request.search.query;

import org.opensearch.index.query.AbstractQueryBuilder;
import org.opensearch.index.query.BoolQueryBuilder;
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.protobufs.BoolQuery;
import org.opensearch.protobufs.MinimumShouldMatch;
import org.opensearch.protobufs.QueryContainer;
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry;

import java.util.List;

/**
* Utility class for converting BoolQuery Protocol Buffers to OpenSearch objects.
* This class provides methods to transform Protocol Buffer representations of bool queries
* into their corresponding OpenSearch BoolQueryBuilder implementations for search operations.
*/
class BoolQueryBuilderProtoUtils {

private BoolQueryBuilderProtoUtils() {
// Utility class, no instances
}

/**
* Converts a Protocol Buffer BoolQuery to an OpenSearch BoolQueryBuilder.
* Similar to {@link BoolQueryBuilder#fromXContent(org.opensearch.core.xcontent.XContentParser)}, this method
* parses the Protocol Buffer representation and creates a properly configured
* BoolQueryBuilder with the appropriate must, must_not, should, filter clauses,
* boost, query name, and minimum_should_match.
*
* @param boolQueryProto The Protocol Buffer BoolQuery object
* @param registry The registry to use for converting nested queries
* @return A configured BoolQueryBuilder instance
*/
static BoolQueryBuilder fromProto(BoolQuery boolQueryProto, QueryBuilderProtoConverterRegistry registry) {
String queryName = null;
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
String minimumShouldMatch = null;
boolean adjustPureNegative = BoolQueryBuilder.ADJUST_PURE_NEGATIVE_DEFAULT;

// Create BoolQueryBuilder
BoolQueryBuilder boolQuery = new BoolQueryBuilder();

// Process name
if (boolQueryProto.hasXName()) {
queryName = boolQueryProto.getXName();
boolQuery.queryName(queryName);
}

// Process boost
if (boolQueryProto.hasBoost()) {
boost = boolQueryProto.getBoost();
boolQuery.boost(boost);
}

// Process minimum_should_match
if (boolQueryProto.hasMinimumShouldMatch()) {
MinimumShouldMatch minimumShouldMatchProto = boolQueryProto.getMinimumShouldMatch();
switch (minimumShouldMatchProto.getMinimumShouldMatchCase()) {
case INT32:
minimumShouldMatch = String.valueOf(minimumShouldMatchProto.getInt32());
break;
case STRING:
minimumShouldMatch = minimumShouldMatchProto.getString();
break;
default:
// No minimum_should_match specified
break;
}

if (minimumShouldMatch != null) {
boolQuery.minimumShouldMatch(minimumShouldMatch);
}
}

// Process must clauses
List<QueryContainer> mustClauses = boolQueryProto.getMustList();
for (QueryContainer queryContainer : mustClauses) {
QueryBuilder queryBuilder = registry.fromProto(queryContainer);
if (queryBuilder != null) {
boolQuery.must(queryBuilder);
}
}

// Process must_not clauses
List<QueryContainer> mustNotClauses = boolQueryProto.getMustNotList();
for (QueryContainer queryContainer : mustNotClauses) {
QueryBuilder queryBuilder = registry.fromProto(queryContainer);
if (queryBuilder != null) {
boolQuery.mustNot(queryBuilder);
}
}

// Process should clauses
List<QueryContainer> shouldClauses = boolQueryProto.getShouldList();
for (QueryContainer queryContainer : shouldClauses) {
QueryBuilder queryBuilder = registry.fromProto(queryContainer);
if (queryBuilder != null) {
boolQuery.should(queryBuilder);
}
}

// Process filter clauses
List<QueryContainer> filterClauses = boolQueryProto.getFilterList();
for (QueryContainer queryContainer : filterClauses) {
QueryBuilder queryBuilder = registry.fromProto(queryContainer);
if (queryBuilder != null) {
boolQuery.filter(queryBuilder);
}
}

// Process adjust_pure_negative
if (boolQueryProto.hasAdjustPureNegative()) {
adjustPureNegative = boolQueryProto.getAdjustPureNegative();
boolQuery.adjustPureNegative(adjustPureNegative);
}

return boolQuery;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ protected void registerBuiltInConverters() {
delegate.registerConverter(new TermsQueryBuilderProtoConverter());
delegate.registerConverter(new MatchPhraseQueryBuilderProtoConverter());
delegate.registerConverter(new MultiMatchQueryBuilderProtoConverter());
delegate.registerConverter(new BoolQueryBuilderProtoConverter());

// Set the registry on all converters so they can access each other
delegate.setRegistryOnAllConverters(this);
logger.info("Registered {} built-in query converters", delegate.size());
}

Expand All @@ -73,4 +76,13 @@ public QueryBuilder fromProto(QueryContainer queryContainer) {
public void registerConverter(QueryBuilderProtoConverter converter) {
delegate.registerConverter(converter);
}

/**
* Updates the registry on all registered converters.
* This should be called after all external converters have been registered
* to ensure converters like BoolQueryBuilderProtoConverter can access the complete registry.
*/
public void updateRegistryOnAllConverters() {
delegate.setRegistryOnAllConverters(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.protobufs.QueryContainer;
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverter;
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry;

import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -71,6 +72,19 @@ public int size() {
return converterMap.size();
}

/**
* Sets the registry on all registered converters.
* This is used to inject the complete registry into converters that need it (like BoolQueryBuilderProtoConverter).
*
* @param registry The registry to inject into all converters
*/
public void setRegistryOnAllConverters(QueryBuilderProtoConverterRegistry registry) {
for (QueryBuilderProtoConverter converter : converterMap.values()) {
converter.setRegistry(registry);
}
logger.info("Set registry on {} converters", converterMap.size());
}

/**
* Registers a new converter.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ public void testCreateComponentsWithExternalConverters() {
Mockito.verify(mockConverter, Mockito.times(3)).getHandledQueryCase();

// Verify that setRegistry was called to inject the registry
Mockito.verify(mockConverter, Mockito.times(1)).setRegistry(Mockito.any());
// Note: setRegistry is called 2 times:
// 1. In createComponents() when processing external converters
// 2. In updateRegistryOnAllConverters() to ensure all converters have the complete registry
Mockito.verify(mockConverter, Mockito.times(2)).setRegistry(Mockito.any());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/
package org.opensearch.transport.grpc.proto.request.search.query;

import org.opensearch.index.query.BoolQueryBuilder;
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.protobufs.BoolQuery;
import org.opensearch.protobufs.QueryContainer;
import org.opensearch.test.OpenSearchTestCase;

public class BoolQueryBuilderProtoConverterTests extends OpenSearchTestCase {

private BoolQueryBuilderProtoConverter converter;

@Override
public void setUp() throws Exception {
super.setUp();
converter = new BoolQueryBuilderProtoConverter();
// Set up the registry for nested query conversion
QueryBuilderProtoConverterRegistryImpl registry = new QueryBuilderProtoConverterRegistryImpl();
converter.setRegistry(registry);
}

public void testGetHandledQueryCase() {
// Test that the converter returns the correct QueryContainerCase
assertEquals("Converter should handle BOOL case", QueryContainer.QueryContainerCase.BOOL, converter.getHandledQueryCase());
}

public void testFromProto() {
// Create a QueryContainer with BoolQuery
BoolQuery boolQuery = BoolQuery.newBuilder().setBoost(2.0f).setXName("test_bool_query").setAdjustPureNegative(false).build();
QueryContainer queryContainer = QueryContainer.newBuilder().setBool(boolQuery).build();

// Convert the query
QueryBuilder queryBuilder = converter.fromProto(queryContainer);

// Verify the result
assertNotNull("QueryBuilder should not be null", queryBuilder);
assertTrue("QueryBuilder should be a BoolQueryBuilder", queryBuilder instanceof BoolQueryBuilder);
BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
assertEquals("Boost should match", 2.0f, boolQueryBuilder.boost(), 0.0f);
assertEquals("Query name should match", "test_bool_query", boolQueryBuilder.queryName());
assertFalse("Adjust pure negative should match", boolQueryBuilder.adjustPureNegative());
}

public void testFromProtoWithMinimalFields() {
// Create a QueryContainer with minimal BoolQuery
BoolQuery boolQuery = BoolQuery.newBuilder().build();
QueryContainer queryContainer = QueryContainer.newBuilder().setBool(boolQuery).build();

// Convert the query
QueryBuilder queryBuilder = converter.fromProto(queryContainer);

// Verify the result
assertNotNull("QueryBuilder should not be null", queryBuilder);
assertTrue("QueryBuilder should be a BoolQueryBuilder", queryBuilder instanceof BoolQueryBuilder);
BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
assertEquals("Default boost should be 1.0", 1.0f, boolQueryBuilder.boost(), 0.0f);
assertNull("Query name should be null", boolQueryBuilder.queryName());
assertTrue("Default adjust pure negative should be true", boolQueryBuilder.adjustPureNegative());
}

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 Bool query'",
exception.getMessage().contains("does not contain a Bool query")
);
}

public void testFromProtoWithNullContainer() {
// Test that the converter throws an exception with null input
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> converter.fromProto(null));

// Verify the exception message
assertTrue(
"Exception message should mention null",
exception.getMessage().contains("null") || exception.getMessage().contains("does not contain a Bool query")
);
}
}
Loading
Loading