Skip to content

Commit 149ec4b

Browse files
luckyxilu66lucy66hw
authored andcommitted
Address comment
Signed-off-by: xil <fridalu66@gmail.com>
1 parent 063d5dd commit 149ec4b

15 files changed

+177
-453
lines changed

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/functionscore/ExpDecayFunctionProtoConverter.java

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import org.opensearch.protobufs.DateDecayPlacement;
1414
import org.opensearch.protobufs.DecayFunction;
1515
import org.opensearch.protobufs.DecayPlacement;
16-
import org.opensearch.protobufs.FunctionScoreContainer;
1716
import org.opensearch.protobufs.GeoDecayPlacement;
1817
import org.opensearch.protobufs.NumericDecayPlacement;
1918
import org.opensearch.transport.grpc.proto.request.common.GeoPointProtoUtils;
2019

20+
import java.util.Map;
21+
2122
/**
2223
* Protocol Buffer converter for ExpDecayFunction.
2324
* This converter handles the transformation of Protocol Buffer ExpDecayFunction objects
@@ -33,42 +34,21 @@ public ExpDecayFunctionProtoConverter() {
3334
}
3435

3536
/**
36-
* Returns the function score container case that this converter handles.
37+
* Converts a Protocol Buffer DecayFunction to an OpenSearch ScoreFunctionBuilder.
38+
* Similar to {@link org.opensearch.index.query.functionscore.ExponentialDecayFunctionBuilder},
39+
* this method parses the Protocol Buffer representation and creates a properly configured
40+
* ExponentialDecayFunctionBuilder with decay placement parameters (numeric, geo, or date).
3741
*
38-
* @return the EXP function score container case
39-
*/
40-
public FunctionScoreContainer.FunctionScoreContainerCase getHandledFunctionCase() {
41-
return FunctionScoreContainer.FunctionScoreContainerCase.EXP;
42-
}
43-
44-
/**
45-
* Converts a Protocol Buffer FunctionScoreContainer containing an exponential decay function
46-
* to an OpenSearch ScoreFunctionBuilder.
47-
*
48-
* @param container the Protocol Buffer FunctionScoreContainer containing the exponential decay function
42+
* @param decayFunction the Protocol Buffer DecayFunction
4943
* @return the corresponding OpenSearch ScoreFunctionBuilder
50-
* @throws IllegalArgumentException if the container is null or doesn't contain an EXP function
51-
*/
52-
public ScoreFunctionBuilder<?> fromProto(FunctionScoreContainer container) {
53-
if (container == null || container.getFunctionScoreContainerCase() != FunctionScoreContainer.FunctionScoreContainerCase.EXP) {
54-
throw new IllegalArgumentException("FunctionScoreContainer must contain an ExpDecayFunction");
55-
}
56-
57-
return parseExpDecayFunction(container.getExp());
58-
}
59-
60-
/**
61-
* Parses a DecayFunction and creates an ExpDecayFunctionBuilder.
62-
*
63-
* @param decayFunction the protobuf DecayFunction
64-
* @return the corresponding OpenSearch ExpDecayFunctionBuilder
44+
* @throws IllegalArgumentException if the decayFunction is null or doesn't contain placements
6545
*/
66-
private static ScoreFunctionBuilder<?> parseExpDecayFunction(DecayFunction decayFunction) {
67-
if (decayFunction.getPlacementCount() == 0) {
46+
public ScoreFunctionBuilder<?> fromProto(DecayFunction decayFunction) {
47+
if (decayFunction == null || decayFunction.getPlacementCount() == 0) {
6848
throw new IllegalArgumentException("DecayFunction must have at least one placement");
6949
}
7050

71-
var entry = decayFunction.getPlacementMap().entrySet().iterator().next();
51+
Map.Entry<String, DecayPlacement> entry = decayFunction.getPlacementMap().entrySet().iterator().next();
7252
String fieldName = entry.getKey();
7353
DecayPlacement decayPlacement = entry.getValue();
7454

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/functionscore/FieldValueFactorFunctionProtoConverter.java

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.opensearch.index.query.functionscore.ScoreFunctionBuilder;
1212
import org.opensearch.protobufs.FieldValueFactorModifier;
1313
import org.opensearch.protobufs.FieldValueFactorScoreFunction;
14-
import org.opensearch.protobufs.FunctionScoreContainer;
1514

1615
/**
1716
* Protocol Buffer converter for FieldValueFactorScoreFunction.
@@ -28,38 +27,15 @@ public FieldValueFactorFunctionProtoConverter() {
2827
}
2928

3029
/**
31-
* Returns the function score container case that this converter handles.
30+
* Converts a Protocol Buffer FieldValueFactorScoreFunction to an OpenSearch ScoreFunctionBuilder.
31+
* Similar to {@link org.opensearch.index.query.functionscore.FieldValueFactorFunctionBuilder#fromXContent},
32+
* this method parses the field, factor, missing value, and modifier parameters.
3233
*
33-
* @return the FIELD_VALUE_FACTOR function score container case
34-
*/
35-
public FunctionScoreContainer.FunctionScoreContainerCase getHandledFunctionCase() {
36-
return FunctionScoreContainer.FunctionScoreContainerCase.FIELD_VALUE_FACTOR;
37-
}
38-
39-
/**
40-
* Converts a Protocol Buffer FunctionScoreContainer containing a field value factor function
41-
* to an OpenSearch ScoreFunctionBuilder.
42-
*
43-
* @param container the Protocol Buffer FunctionScoreContainer containing the field value factor function
34+
* @param fieldValueFactor the Protocol Buffer FieldValueFactorScoreFunction
4435
* @return the corresponding OpenSearch ScoreFunctionBuilder
45-
* @throws IllegalArgumentException if the container is null or doesn't contain a FIELD_VALUE_FACTOR function
46-
*/
47-
public ScoreFunctionBuilder<?> fromProto(FunctionScoreContainer container) {
48-
if (container == null
49-
|| container.getFunctionScoreContainerCase() != FunctionScoreContainer.FunctionScoreContainerCase.FIELD_VALUE_FACTOR) {
50-
throw new IllegalArgumentException("FunctionScoreContainer must contain a FieldValueFactorScoreFunction");
51-
}
52-
53-
return parseFieldValueFactorFunction(container.getFieldValueFactor());
54-
}
55-
56-
/**
57-
* Parses a FieldValueFactorScoreFunction and creates a FieldValueFactorFunctionBuilder.
58-
*
59-
* @param fieldValueFactor the protobuf FieldValueFactorScoreFunction
60-
* @return the corresponding OpenSearch FieldValueFactorFunctionBuilder
36+
* @throws IllegalArgumentException if the fieldValueFactor is null
6137
*/
62-
private static FieldValueFactorFunctionBuilder parseFieldValueFactorFunction(FieldValueFactorScoreFunction fieldValueFactor) {
38+
public ScoreFunctionBuilder<?> fromProto(FieldValueFactorScoreFunction fieldValueFactor) {
6339
if (fieldValueFactor == null) {
6440
throw new IllegalArgumentException("FieldValueFactorScoreFunction cannot be null");
6541
}

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/functionscore/FunctionScoreQueryBuilderProtoConverter.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ public FunctionScoreQueryBuilderProtoConverter() {}
2929
@Override
3030
public void setRegistry(QueryBuilderProtoConverterRegistry registry) {
3131
this.registry = registry;
32-
// Pass the registry to the utility class so it can convert nested queries
33-
FunctionScoreQueryBuilderProtoUtils.setRegistry(registry);
3432
}
3533

3634
@Override
@@ -44,6 +42,6 @@ public QueryBuilder fromProto(QueryContainer queryContainer) {
4442
throw new IllegalArgumentException("QueryContainer does not contain a FunctionScore query");
4543
}
4644

47-
return FunctionScoreQueryBuilderProtoUtils.fromProto(queryContainer.getFunctionScore());
45+
return FunctionScoreQueryBuilderProtoUtils.fromProto(queryContainer.getFunctionScore(), registry);
4846
}
4947
}

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/functionscore/FunctionScoreQueryBuilderProtoUtils.java

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,10 @@
3131
*/
3232
class FunctionScoreQueryBuilderProtoUtils {
3333

34-
// Registry for query conversion - injected by the gRPC plugin
35-
private static QueryBuilderProtoConverterRegistry REGISTRY;
36-
3734
private FunctionScoreQueryBuilderProtoUtils() {
3835
// Utility class, no instances
3936
}
4037

41-
/**
42-
* Sets the registry injected by the gRPC plugin.
43-
* This method is called when the FunctionScore converter receives the populated registry.
44-
*
45-
* @param registry The registry to use
46-
*/
47-
public static void setRegistry(QueryBuilderProtoConverterRegistry registry) {
48-
REGISTRY = registry;
49-
}
50-
5138
/**
5239
* Converts a Protocol Buffer FunctionScoreQuery to an OpenSearch FunctionScoreQueryBuilder.
5340
* Similar to {@link FunctionScoreQueryBuilder#fromXContent(XContentParser)}, this method
@@ -56,10 +43,11 @@ public static void setRegistry(QueryBuilderProtoConverterRegistry registry) {
5643
* max boost, min score, boost, and query name settings.
5744
*
5845
* @param functionScoreQueryProto The Protocol Buffer FunctionScoreQuery object
46+
* @param registry The registry to use for converting nested queries
5947
* @return A configured FunctionScoreQueryBuilder instance
6048
* @throws IllegalArgumentException if the function score query is invalid or contains unsupported function types
6149
*/
62-
public static FunctionScoreQueryBuilder fromProto(FunctionScoreQuery functionScoreQueryProto) {
50+
static FunctionScoreQueryBuilder fromProto(FunctionScoreQuery functionScoreQueryProto, QueryBuilderProtoConverterRegistry registry) {
6351
if (functionScoreQueryProto == null) {
6452
throw new IllegalArgumentException("FunctionScoreQuery cannot be null");
6553
}
@@ -77,12 +65,12 @@ public static FunctionScoreQueryBuilder fromProto(FunctionScoreQuery functionSco
7765

7866
if (functionScoreQueryProto.hasQuery()) {
7967
QueryContainer queryContainer = functionScoreQueryProto.getQuery();
80-
query = REGISTRY.fromProto(queryContainer);
68+
query = registry.fromProto(queryContainer);
8169
}
8270

8371
if (functionScoreQueryProto.getFunctionsCount() > 0) {
8472
for (FunctionScoreContainer container : functionScoreQueryProto.getFunctionsList()) {
85-
FunctionScoreQueryBuilder.FilterFunctionBuilder filterFunctionBuilder = parseFunctionScoreContainer(container);
73+
FunctionScoreQueryBuilder.FilterFunctionBuilder filterFunctionBuilder = parseFunctionScoreContainer(container, registry);
8674
filterFunctionBuilders.add(filterFunctionBuilder);
8775
}
8876
}
@@ -96,7 +84,8 @@ public static FunctionScoreQueryBuilder fromProto(FunctionScoreQuery functionSco
9684
filterFunctionBuilders.toArray(new FunctionScoreQueryBuilder.FilterFunctionBuilder[0])
9785
);
9886

99-
if (functionScoreQueryProto.hasBoostMode()) {
87+
if (functionScoreQueryProto.hasBoostMode()
88+
&& functionScoreQueryProto.getBoostMode() != FunctionBoostMode.FUNCTION_BOOST_MODE_UNSPECIFIED) {
10089
combineFunction = parseBoostMode(functionScoreQueryProto.getBoostMode());
10190
}
10291

@@ -139,7 +128,10 @@ public static FunctionScoreQueryBuilder fromProto(FunctionScoreQuery functionSco
139128
/**
140129
* Parses a FunctionScoreContainer and creates the appropriate FilterFunctionBuilder.
141130
*/
142-
private static FunctionScoreQueryBuilder.FilterFunctionBuilder parseFunctionScoreContainer(FunctionScoreContainer container) {
131+
private static FunctionScoreQueryBuilder.FilterFunctionBuilder parseFunctionScoreContainer(
132+
FunctionScoreContainer container,
133+
QueryBuilderProtoConverterRegistry registry
134+
) {
143135
if (container == null) {
144136
throw new IllegalArgumentException("FunctionScoreContainer cannot be null");
145137
}
@@ -149,7 +141,7 @@ private static FunctionScoreQueryBuilder.FilterFunctionBuilder parseFunctionScor
149141
Float functionWeight = null;
150142
if (container.hasFilter()) {
151143
QueryContainer filterContainer = container.getFilter();
152-
filter = REGISTRY.fromProto(filterContainer);
144+
filter = registry.fromProto(filterContainer);
153145
}
154146

155147
// Check for weight (only set if present, otherwise use default)
@@ -191,12 +183,12 @@ private static ScoreFunctionBuilder<?> parseScoreFunction(FunctionScoreContainer
191183
FunctionScoreContainer.FunctionScoreContainerCase functionCase = container.getFunctionScoreContainerCase();
192184

193185
return switch (functionCase) {
194-
case FIELD_VALUE_FACTOR -> new FieldValueFactorFunctionProtoConverter().fromProto(container);
195-
case RANDOM_SCORE -> new RandomScoreFunctionProtoConverter().fromProto(container);
196-
case SCRIPT_SCORE -> new ScriptScoreFunctionProtoConverter().fromProto(container);
197-
case EXP -> new ExpDecayFunctionProtoConverter().fromProto(container);
198-
case GAUSS -> new GaussDecayFunctionProtoConverter().fromProto(container);
199-
case LINEAR -> new LinearDecayFunctionProtoConverter().fromProto(container);
186+
case FIELD_VALUE_FACTOR -> new FieldValueFactorFunctionProtoConverter().fromProto(container.getFieldValueFactor());
187+
case RANDOM_SCORE -> new RandomScoreFunctionProtoConverter().fromProto(container.getRandomScore());
188+
case SCRIPT_SCORE -> new ScriptScoreFunctionProtoConverter().fromProto(container.getScriptScore());
189+
case EXP -> new ExpDecayFunctionProtoConverter().fromProto(container.getExp());
190+
case GAUSS -> new GaussDecayFunctionProtoConverter().fromProto(container.getGauss());
191+
case LINEAR -> new LinearDecayFunctionProtoConverter().fromProto(container.getLinear());
200192
default -> throw new IllegalArgumentException("Unsupported function score type: " + functionCase);
201193
};
202194
}
@@ -219,7 +211,7 @@ private static CombineFunction parseBoostMode(FunctionBoostMode boostMode) {
219211
case FUNCTION_BOOST_MODE_SUM:
220212
return CombineFunction.SUM;
221213
default:
222-
return FunctionScoreQueryBuilder.DEFAULT_BOOST_MODE;
214+
throw new IllegalArgumentException("Unsupported boost mode: " + boostMode);
223215
}
224216
}
225217

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/functionscore/GaussDecayFunctionProtoConverter.java

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import org.opensearch.protobufs.DateDecayPlacement;
1414
import org.opensearch.protobufs.DecayFunction;
1515
import org.opensearch.protobufs.DecayPlacement;
16-
import org.opensearch.protobufs.FunctionScoreContainer;
1716
import org.opensearch.protobufs.GeoDecayPlacement;
1817
import org.opensearch.protobufs.NumericDecayPlacement;
1918
import org.opensearch.transport.grpc.proto.request.common.GeoPointProtoUtils;
2019

20+
import java.util.Map;
21+
2122
/**
2223
* Protocol Buffer converter for GaussDecayFunction.
2324
* This converter handles the transformation of Protocol Buffer GaussDecayFunction objects
@@ -33,42 +34,21 @@ public GaussDecayFunctionProtoConverter() {
3334
}
3435

3536
/**
36-
* Returns the function score container case that this converter handles.
37+
* Converts a Protocol Buffer DecayFunction to an OpenSearch ScoreFunctionBuilder.
38+
* Similar to {@link org.opensearch.index.query.functionscore.GaussDecayFunctionBuilder},
39+
* this method parses the Protocol Buffer representation and creates a properly configured
40+
* GaussDecayFunctionBuilder with decay placement parameters (numeric, geo, or date).
3741
*
38-
* @return the GAUSS function score container case
39-
*/
40-
public FunctionScoreContainer.FunctionScoreContainerCase getHandledFunctionCase() {
41-
return FunctionScoreContainer.FunctionScoreContainerCase.GAUSS;
42-
}
43-
44-
/**
45-
* Converts a Protocol Buffer FunctionScoreContainer containing a Gaussian decay function
46-
* to an OpenSearch ScoreFunctionBuilder.
47-
*
48-
* @param container the Protocol Buffer FunctionScoreContainer containing the Gaussian decay function
42+
* @param decayFunction the Protocol Buffer DecayFunction
4943
* @return the corresponding OpenSearch ScoreFunctionBuilder
50-
* @throws IllegalArgumentException if the container is null or doesn't contain a GAUSS function
51-
*/
52-
public ScoreFunctionBuilder<?> fromProto(FunctionScoreContainer container) {
53-
if (container == null || container.getFunctionScoreContainerCase() != FunctionScoreContainer.FunctionScoreContainerCase.GAUSS) {
54-
throw new IllegalArgumentException("FunctionScoreContainer must contain a GaussDecayFunction");
55-
}
56-
57-
return parseGaussDecayFunction(container.getGauss());
58-
}
59-
60-
/**
61-
* Parses a DecayFunction and creates a GaussDecayFunctionBuilder.
62-
*
63-
* @param decayFunction the protobuf DecayFunction
64-
* @return the corresponding OpenSearch GaussDecayFunctionBuilder
44+
* @throws IllegalArgumentException if the decayFunction is null or doesn't contain placements
6545
*/
66-
private static ScoreFunctionBuilder<?> parseGaussDecayFunction(DecayFunction decayFunction) {
67-
if (decayFunction.getPlacementCount() == 0) {
46+
public ScoreFunctionBuilder<?> fromProto(DecayFunction decayFunction) {
47+
if (decayFunction == null || decayFunction.getPlacementCount() == 0) {
6848
throw new IllegalArgumentException("DecayFunction must have at least one placement");
6949
}
7050

71-
var entry = decayFunction.getPlacementMap().entrySet().iterator().next();
51+
Map.Entry<String, DecayPlacement> entry = decayFunction.getPlacementMap().entrySet().iterator().next();
7252
String fieldName = entry.getKey();
7353
DecayPlacement decayPlacement = entry.getValue();
7454

0 commit comments

Comments
 (0)