Skip to content

Commit e709b11

Browse files
karenyrxkarenyrx
committed
Make QueryBuilderProtoConverter and QueryBuilderProtoConverterRegistry an SPI
1 parent c01ff89 commit e709b11

File tree

6 files changed

+194
-84
lines changed

6 files changed

+194
-84
lines changed

‎modules/transport-grpc/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ testClusters {
2121
}
2222

2323
dependencies {
24+
api project('spi')
2425
compileOnly "com.google.code.findbugs:jsr305:3.0.2"
2526
runtimeOnly "com.google.guava:guava:${versions.guava}"
2627
implementation "com.google.errorprone:error_prone_annotations:2.24.1"

‎modules/transport-grpc/spi/build.gradle‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
apply plugin: 'opensearch.build'
10+
apply plugin: 'opensearch.publish'
11+
12+
base {
13+
group = 'org.opensearch.plugin'
14+
archivesName = 'transport-grpc-spi'
15+
}
16+
17+
dependencies {
18+
api project(":server")
19+
api "org.opensearch:protobufs:0.6.0"
20+
}
21+
22+
// no tests...yet?
23+
test.enabled = false

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverter.java‎ renamed to ‎modules/transport-grpc/spi/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverter.java‎

File renamed without changes.

‎modules/transport-grpc/spi/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterSpiRegistry.java‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
package org.opensearch.transport.grpc.proto.request.search.query;
9+
10+
import org.apache.logging.log4j.LogManager;
11+
import org.apache.logging.log4j.Logger;
12+
import org.opensearch.common.inject.Inject;
13+
import org.opensearch.common.inject.Singleton;
14+
import org.opensearch.index.query.QueryBuilder;
15+
import org.opensearch.protobufs.QueryContainer;
16+
17+
import java.util.HashMap;
18+
import java.util.Iterator;
19+
import java.util.Map;
20+
import java.util.ServiceLoader;
21+
22+
/**
23+
* Registry for QueryBuilderProtoConverter implementations.
24+
* This class discovers and manages all available converters.
25+
*/
26+
@Singleton
27+
public class QueryBuilderProtoConverterSpiRegistry {
28+
29+
private static final Logger logger = LogManager.getLogger(QueryBuilderProtoConverterSpiRegistry.class);
30+
private final Map<QueryContainer.QueryContainerCase, QueryBuilderProtoConverter> converterMap = new HashMap<>();
31+
32+
/**
33+
* Creates a new registry and loads all available converters.
34+
*/
35+
@Inject
36+
public QueryBuilderProtoConverterSpiRegistry() {
37+
// Discover converters from plugins using Java's ServiceLoader
38+
loadExternalConverters();
39+
}
40+
41+
/**
42+
* Loads external converters using Java's ServiceLoader mechanism.
43+
* Protected for testing purposes.
44+
*/
45+
protected void loadExternalConverters() {
46+
ServiceLoader<QueryBuilderProtoConverter> serviceLoader = ServiceLoader.load(QueryBuilderProtoConverter.class);
47+
Iterator<QueryBuilderProtoConverter> iterator = serviceLoader.iterator();
48+
49+
int count = 0;
50+
int failedCount = 0;
51+
while (iterator.hasNext()) {
52+
try {
53+
QueryBuilderProtoConverter converter = iterator.next();
54+
registerConverter(converter);
55+
count++;
56+
logger.info("Loaded external query converter for {}: {}", converter.getHandledQueryCase(), converter.getClass().getName());
57+
} catch (Exception e) {
58+
failedCount++;
59+
logger.error("Failed to load external query converter", e);
60+
}
61+
}
62+
63+
logger.info("Loaded {} external query converters ({} failed)", count, failedCount);
64+
}
65+
66+
/**
67+
* Converts a protobuf query container to an OpenSearch QueryBuilder.
68+
*
69+
* @param queryContainer The protobuf query container
70+
* @return The corresponding OpenSearch QueryBuilder
71+
* @throws IllegalArgumentException if no converter can handle the query
72+
*/
73+
public QueryBuilder fromProto(QueryContainer queryContainer) {
74+
if (queryContainer == null) {
75+
throw new IllegalArgumentException("Query container cannot be null");
76+
}
77+
78+
// Use direct map lookup for better performance
79+
QueryContainer.QueryContainerCase queryCase = queryContainer.getQueryContainerCase();
80+
QueryBuilderProtoConverter converter = converterMap.get(queryCase);
81+
82+
if (converter != null) {
83+
logger.debug("Using converter for {}: {}", queryCase, converter.getClass().getName());
84+
return converter.fromProto(queryContainer);
85+
}
86+
87+
throw new IllegalArgumentException("Unsupported query type in container: " + queryContainer + " (case: " + queryCase + ")");
88+
}
89+
90+
/**
91+
* Gets the number of registered converters.
92+
*
93+
* @return The number of registered converters
94+
*/
95+
public int size() {
96+
return converterMap.size();
97+
}
98+
99+
/**
100+
* Registers a new converter.
101+
*
102+
* @param converter The converter to register
103+
* @throws IllegalArgumentException if the converter is null or its handled query case is invalid
104+
*/
105+
public void registerConverter(QueryBuilderProtoConverter converter) {
106+
if (converter == null) {
107+
throw new IllegalArgumentException("Converter cannot be null");
108+
}
109+
110+
QueryContainer.QueryContainerCase queryCase = converter.getHandledQueryCase();
111+
112+
if (queryCase == null) {
113+
throw new IllegalArgumentException("Handled query case cannot be null for converter: " + converter.getClass().getName());
114+
}
115+
116+
if (queryCase == QueryContainer.QueryContainerCase.QUERYCONTAINER_NOT_SET) {
117+
throw new IllegalArgumentException(
118+
"Cannot register converter for QUERYCONTAINER_NOT_SET case: " + converter.getClass().getName()
119+
);
120+
}
121+
122+
QueryBuilderProtoConverter existingConverter = converterMap.put(queryCase, converter);
123+
if (existingConverter != null) {
124+
logger.warn(
125+
"Replacing existing converter for query type {}: {} -> {}",
126+
queryCase,
127+
existingConverter.getClass().getName(),
128+
converter.getClass().getName()
129+
);
130+
}
131+
132+
logger.debug("Registered query converter for {}: {}", queryCase, converter.getClass().getName());
133+
}
134+
}

‎modules/transport-grpc/spi/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/package-info.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
/**
10+
* Service Provider Interface (SPI) for transport-grpc query builder converters.
11+
*
12+
* This package provides the core interfaces that external plugins can implement
13+
* to add custom query types to the gRPC transport layer. The primary interface
14+
* is {@link org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter}
15+
* which allows conversion from protobuf query containers to OpenSearch QueryBuilder objects.
16+
*
17+
* External plugins should implement QueryBuilderProtoConverter and register their
18+
* implementations via Java's ServiceLoader mechanism by including their implementation
19+
* class names in META-INF/services/org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverter
20+
*/
21+
package org.opensearch.transport.grpc.proto.request.search.query;

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistry.java‎

Lines changed: 15 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -13,32 +13,28 @@
1313
import org.opensearch.common.inject.Singleton;
1414
import org.opensearch.index.query.QueryBuilder;
1515
import org.opensearch.protobufs.QueryContainer;
16-
17-
import java.util.HashMap;
18-
import java.util.Iterator;
19-
import java.util.Map;
20-
import java.util.ServiceLoader;
16+
import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoConverterSpiRegistry;
2117

2218
/**
2319
* Registry for QueryBuilderProtoConverter implementations.
24-
* This class discovers and manages all available converters.
20+
* This class wraps the SPI registry and adds built-in converters for the transport-grpc module.
2521
*/
2622
@Singleton
2723
public class QueryBuilderProtoConverterRegistry {
2824

2925
private static final Logger logger = LogManager.getLogger(QueryBuilderProtoConverterRegistry.class);
30-
private final Map<QueryContainer.QueryContainerCase, QueryBuilderProtoConverter> converterMap = new HashMap<>();
26+
private final QueryBuilderProtoConverterSpiRegistry delegate;
3127

3228
/**
3329
* Creates a new registry and loads all available converters.
3430
*/
3531
@Inject
3632
public QueryBuilderProtoConverterRegistry() {
37-
// Load built-in converters
38-
registerBuiltInConverters();
33+
// Create the SPI registry which loads external converters
34+
this.delegate = new QueryBuilderProtoConverterSpiRegistry();
3935

40-
// Discover converters from other plugins using Java's ServiceLoader
41-
loadExternalConverters();
36+
// Register built-in converters for this module
37+
registerBuiltInConverters();
4238
}
4339

4440
/**
@@ -47,96 +43,31 @@ public QueryBuilderProtoConverterRegistry() {
4743
*/
4844
protected void registerBuiltInConverters() {
4945
// Add built-in converters
50-
registerConverter(new MatchAllQueryBuilderProtoConverter());
51-
registerConverter(new MatchNoneQueryBuilderProtoConverter());
52-
registerConverter(new TermQueryBuilderProtoConverter());
53-
registerConverter(new TermsQueryBuilderProtoConverter());
54-
55-
logger.info("Registered {} built-in query converters", converterMap.size());
56-
}
57-
58-
/**
59-
* Loads external converters using Java's ServiceLoader mechanism.
60-
* Protected for testing purposes.
61-
*/
62-
protected void loadExternalConverters() {
63-
ServiceLoader<QueryBuilderProtoConverter> serviceLoader = ServiceLoader.load(QueryBuilderProtoConverter.class);
64-
Iterator<QueryBuilderProtoConverter> iterator = serviceLoader.iterator();
46+
delegate.registerConverter(new MatchAllQueryBuilderProtoConverter());
47+
delegate.registerConverter(new MatchNoneQueryBuilderProtoConverter());
48+
delegate.registerConverter(new TermQueryBuilderProtoConverter());
49+
delegate.registerConverter(new TermsQueryBuilderProtoConverter());
6550

66-
int count = 0;
67-
int failedCount = 0;
68-
while (iterator.hasNext()) {
69-
try {
70-
QueryBuilderProtoConverter converter = iterator.next();
71-
registerConverter(converter);
72-
count++;
73-
logger.info("Loaded external query converter for {}: {}", converter.getHandledQueryCase(), converter.getClass().getName());
74-
} catch (Exception e) {
75-
failedCount++;
76-
logger.error("Failed to load external query converter", e);
77-
}
78-
}
79-
80-
logger.info("Loaded {} external query converters ({} failed)", count, failedCount);
51+
logger.info("Registered {} built-in query converters", delegate.size());
8152
}
8253

8354
/**
8455
* Converts a protobuf query container to an OpenSearch QueryBuilder.
8556
*
8657
* @param queryContainer The protobuf query container
8758
* @return The corresponding OpenSearch QueryBuilder
88-
* @throws IllegalArgumentException if no converter can handle the query
59+
* @throws IllegalArgumentException if the query cannot be converted
8960
*/
9061
public QueryBuilder fromProto(QueryContainer queryContainer) {
91-
if (queryContainer == null) {
92-
throw new IllegalArgumentException("Query container cannot be null");
93-
}
94-
95-
// Use direct map lookup for better performance
96-
QueryContainer.QueryContainerCase queryCase = queryContainer.getQueryContainerCase();
97-
QueryBuilderProtoConverter converter = converterMap.get(queryCase);
98-
99-
if (converter != null) {
100-
logger.debug("Using converter for {}: {}", queryCase, converter.getClass().getName());
101-
return converter.fromProto(queryContainer);
102-
}
103-
104-
throw new IllegalArgumentException("Unsupported query type in container: " + queryContainer + " (case: " + queryCase + ")");
62+
return delegate.fromProto(queryContainer);
10563
}
10664

10765
/**
10866
* Registers a new converter.
10967
*
11068
* @param converter The converter to register
111-
* @throws IllegalArgumentException if the converter is null or its handled query case is invalid
11269
*/
11370
public void registerConverter(QueryBuilderProtoConverter converter) {
114-
if (converter == null) {
115-
throw new IllegalArgumentException("Converter cannot be null");
116-
}
117-
118-
QueryContainer.QueryContainerCase queryCase = converter.getHandledQueryCase();
119-
120-
if (queryCase == null) {
121-
throw new IllegalArgumentException("Handled query case cannot be null for converter: " + converter.getClass().getName());
122-
}
123-
124-
if (queryCase == QueryContainer.QueryContainerCase.QUERYCONTAINER_NOT_SET) {
125-
throw new IllegalArgumentException(
126-
"Cannot register converter for QUERYCONTAINER_NOT_SET case: " + converter.getClass().getName()
127-
);
128-
}
129-
130-
QueryBuilderProtoConverter existingConverter = converterMap.put(queryCase, converter);
131-
if (existingConverter != null) {
132-
logger.warn(
133-
"Replacing existing converter for query type {}: {} -> {}",
134-
queryCase,
135-
existingConverter.getClass().getName(),
136-
converter.getClass().getName()
137-
);
138-
}
139-
140-
logger.debug("Registered query converter for {}: {}", queryCase, converter.getClass().getName());
71+
delegate.registerConverter(converter);
14172
}
14273
}

0 commit comments

Comments
 (0)