Skip to content

Commit b3ff223

Browse files
pawankartik-elasticpawankartik-elastic
committed
Move MRT parsing to a util method
1 parent 06b880b commit b3ff223

File tree

8 files changed

+92
-88
lines changed

8 files changed

+92
-88
lines changed

‎modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import java.io.IOException;
2525
import java.util.List;
26+
import java.util.Optional;
2627
import java.util.Set;
2728

2829
import static org.elasticsearch.rest.RestRequest.Method.GET;
@@ -93,7 +94,7 @@ public MultiSearchTemplateRequest parseRequest(RestRequest restRequest, boolean
9394
}
9495
RestSearchAction.validateSearchRequest(restRequest, searchRequest);
9596
},
96-
crossProjectEnabled
97+
Optional.of(crossProjectEnabled)
9798
);
9899
return multiRequest;
99100
}

‎server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Locale;
3737
import java.util.Map;
3838
import java.util.Objects;
39+
import java.util.Optional;
3940
import java.util.stream.Collectors;
4041

4142
import static org.elasticsearch.action.ValidateActions.addValidationError;
@@ -167,7 +168,8 @@ public static void readMultiLineFormat(
167168
String routing,
168169
String searchType,
169170
Boolean ccsMinimizeRoundtrips,
170-
boolean allowExplicitIndex
171+
boolean allowExplicitIndex,
172+
Optional<Boolean> crossProjectEnabled
171173
) throws IOException {
172174
readMultiLineFormat(
173175
xContent,
@@ -180,7 +182,8 @@ public static void readMultiLineFormat(
180182
searchType,
181183
ccsMinimizeRoundtrips,
182184
allowExplicitIndex,
183-
(s, o, r) -> false
185+
(s, o, r) -> false,
186+
crossProjectEnabled
184187
);
185188

186189
}
@@ -196,7 +199,8 @@ public static void readMultiLineFormat(
196199
String searchType,
197200
Boolean ccsMinimizeRoundtrips,
198201
boolean allowExplicitIndex,
199-
TriFunction<String, Object, SearchRequest, Boolean> extraParamParser
202+
TriFunction<String, Object, SearchRequest, Boolean> extraParamParser,
203+
Optional<Boolean> crossProjectEnabled
200204
) throws IOException {
201205
int from = 0;
202206
byte marker = xContent.bulkSeparator();
@@ -219,6 +223,7 @@ public static void readMultiLineFormat(
219223
if (searchType != null) {
220224
searchRequest.searchType(searchType);
221225
}
226+
// When crossProjectEnabled is true, ccsMinimizeRoundtrips is guaranteed to be true.
222227
if (ccsMinimizeRoundtrips != null) {
223228
searchRequest.setCcsMinimizeRoundtrips(ccsMinimizeRoundtrips);
224229
}
@@ -250,7 +255,7 @@ public static void readMultiLineFormat(
250255
} else if ("search_type".equals(entry.getKey()) || "searchType".equals(entry.getKey())) {
251256
searchRequest.searchType(nodeStringValue(value, null));
252257
} else if ("ccs_minimize_roundtrips".equals(entry.getKey()) || "ccsMinimizeRoundtrips".equals(entry.getKey())) {
253-
searchRequest.setCcsMinimizeRoundtrips(nodeBooleanValue(value));
258+
searchRequest.setCcsMinimizeRoundtrips(crossProjectEnabled.orElse(false) || nodeBooleanValue(value));
254259
} else if ("request_cache".equals(entry.getKey()) || "requestCache".equals(entry.getKey())) {
255260
searchRequest.requestCache(nodeBooleanValue(value, entry.getKey()));
256261
} else if ("preference".equals(entry.getKey())) {

‎server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.elasticsearch.common.Strings;
1919
import org.elasticsearch.common.TriFunction;
2020
import org.elasticsearch.common.bytes.ReleasableBytesReference;
21-
import org.elasticsearch.common.logging.HeaderWarning;
2221
import org.elasticsearch.common.settings.Settings;
2322
import org.elasticsearch.core.Tuple;
2423
import org.elasticsearch.features.NodeFeature;
@@ -37,6 +36,7 @@
3736

3837
import java.io.IOException;
3938
import java.util.List;
39+
import java.util.Optional;
4040
import java.util.Set;
4141
import java.util.function.Predicate;
4242

@@ -91,7 +91,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
9191
allowExplicitIndex,
9292
searchUsageHolder,
9393
clusterSupportsFeature,
94-
crossProjectEnabled
94+
Optional.of(crossProjectEnabled)
9595
);
9696
return channel -> {
9797
final RestCancellableNodeClient cancellableClient = new RestCancellableNodeClient(client, request.getHttpChannel());
@@ -111,7 +111,7 @@ public static MultiSearchRequest parseRequest(
111111
boolean allowExplicitIndex,
112112
SearchUsageHolder searchUsageHolder,
113113
Predicate<NodeFeature> clusterSupportsFeature,
114-
boolean crossProjectEnabled
114+
Optional<Boolean> crossProjectEnabled
115115
) throws IOException {
116116
return parseRequest(
117117
restRequest,
@@ -133,7 +133,7 @@ public static MultiSearchRequest parseRequest(
133133
SearchUsageHolder searchUsageHolder,
134134
Predicate<NodeFeature> clusterSupportsFeature,
135135
TriFunction<String, Object, SearchRequest, Boolean> extraParamParser,
136-
boolean crossProjectEnabled
136+
Optional<Boolean> crossProjectEnabled
137137
) throws IOException {
138138
MultiSearchRequest multiRequest = new MultiSearchRequest();
139139
IndicesOptions indicesOptions = IndicesOptions.fromRequest(restRequest, multiRequest.indicesOptions());
@@ -162,7 +162,7 @@ public static MultiSearchRequest parseRequest(
162162
if (searchRequest.pointInTimeBuilder() != null) {
163163
RestSearchAction.preparePointInTime(searchRequest, restRequest);
164164
} else {
165-
searchRequest.setCcsMinimizeRoundtrips(maybeConsumeCcsMrtParam(restRequest, crossProjectEnabled));
165+
searchRequest.setCcsMinimizeRoundtrips(SearchParamsParser.parseCcsMinimizeRoundtrips(crossProjectEnabled, restRequest));
166166
}
167167
multiRequest.add(searchRequest);
168168
}, extraParamParser, crossProjectEnabled);
@@ -187,7 +187,7 @@ public static void parseMultiLineRequest(
187187
IndicesOptions indicesOptions,
188188
boolean allowExplicitIndex,
189189
CheckedBiConsumer<SearchRequest, XContentParser, IOException> consumer,
190-
boolean crossProjectEnabled
190+
Optional<Boolean> crossProjectEnabled
191191
) throws IOException {
192192
parseMultiLineRequest(request, indicesOptions, allowExplicitIndex, consumer, (k, v, r) -> false, crossProjectEnabled);
193193
}
@@ -202,12 +202,12 @@ public static void parseMultiLineRequest(
202202
boolean allowExplicitIndex,
203203
CheckedBiConsumer<SearchRequest, XContentParser, IOException> consumer,
204204
TriFunction<String, Object, SearchRequest, Boolean> extraParamParser,
205-
boolean crossProjectEnabled
205+
Optional<Boolean> crossProjectEnabled
206206
) throws IOException {
207207

208208
String[] indices = Strings.splitStringByCommaToArray(request.param("index"));
209209
String searchType = request.param("search_type");
210-
boolean ccsMinimizeRoundtrips = maybeConsumeCcsMrtParam(request, crossProjectEnabled);
210+
boolean ccsMinimizeRoundtrips = SearchParamsParser.parseCcsMinimizeRoundtrips(crossProjectEnabled, request);
211211
String routing = request.param("routing");
212212

213213
final Tuple<XContentType, ReleasableBytesReference> sourceTuple = request.contentOrSourceParam();
@@ -224,30 +224,11 @@ public static void parseMultiLineRequest(
224224
searchType,
225225
ccsMinimizeRoundtrips,
226226
allowExplicitIndex,
227-
extraParamParser
227+
extraParamParser,
228+
crossProjectEnabled
228229
);
229230
}
230231

231-
private static boolean maybeConsumeCcsMrtParam(RestRequest request, boolean crossProjectEnabled) {
232-
if (crossProjectEnabled) {
233-
// Warn user, consume param, and return true.
234-
if (request.hasParam("ccs_minimize_roundtrips")) {
235-
HeaderWarning.addWarning(RestMultiSearchAction.MRT_ENABLED_IN_CPS_WARN);
236-
request.param("ccs_minimize_roundtrips");
237-
return true;
238-
}
239-
240-
/*
241-
* User has no preference, use the default value that's appropriate for CPS.
242-
* CPS searches should minimise round trips.
243-
*/
244-
return true;
245-
} else {
246-
// Not in CPS environment, pick whatever user chose.
247-
return request.paramAsBoolean("ccs_minimize_roundtrips", true);
248-
}
249-
}
250-
251232
@Override
252233
public boolean mediaTypesValid(RestRequest request) {
253234
return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());

‎server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.elasticsearch.action.support.IndicesOptions;
1717
import org.elasticsearch.client.internal.node.NodeClient;
1818
import org.elasticsearch.common.Strings;
19-
import org.elasticsearch.common.logging.HeaderWarning;
2019
import org.elasticsearch.common.settings.Settings;
2120
import org.elasticsearch.core.Booleans;
2221
import org.elasticsearch.core.Nullable;
@@ -275,33 +274,7 @@ public static void parseSearchRequest(
275274
if (searchRequest.pointInTimeBuilder() != null) {
276275
preparePointInTime(searchRequest, request);
277276
} else {
278-
if (crossProjectEnabled.orElse(false)) {
279-
/*
280-
* MRT should not be settable by the user when in Cross Project Search environment.
281-
* Only _async_search uses MRT=false. However, in RestSubmitAsyncSearchAction, we
282-
* already, explicitly, and directly call in `SearchRequest#setCcsMinimizeRoundtrips()`
283-
* to set it to true. Although other searches that utilise SearchRequest-s do not call
284-
* this method, SearchRequest, by default, sets MRT to true when it is instantiated.
285-
* This way, all searches pivot to MRT=true for CPS.
286-
*
287-
* Since users will anyway see a banner via Kibana that setting MRT in Serverless has no
288-
* effect, we can safely drop it.
289-
*/
290-
if (request.hasParam("ccs_minimize_roundtrips")) {
291-
String warning = "ccs_minimize_roundtrips always defaults to true in Cross Project Search context."
292-
+ " Setting it explicitly has no effect irrespective of the value specified and is ignored.";
293-
HeaderWarning.addWarning(warning);
294-
request.param("ccs_minimize_roundtrips");
295-
}
296-
} else {
297-
/*
298-
* Either we're not in a Cross Project Search environment or the endpoint isn't compatible with it. Parse what's in the
299-
* request.
300-
*/
301-
searchRequest.setCcsMinimizeRoundtrips(
302-
request.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
303-
);
304-
}
277+
searchRequest.setCcsMinimizeRoundtrips(SearchParamsParser.parseCcsMinimizeRoundtrips(crossProjectEnabled, request));
305278
}
306279
if (request.paramAsBoolean("force_synthetic_source", false)) {
307280
searchRequest.setForceSyntheticSource(true);

‎server/src/main/java/org/elasticsearch/rest/action/search/SearchParamsParser.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.rest.action.search;
11+
12+
import org.elasticsearch.common.logging.HeaderWarning;
13+
import org.elasticsearch.rest.RestRequest;
14+
15+
import java.util.Optional;
16+
17+
public class SearchParamsParser {
18+
private static final String MRT_SET_IN_CPS_WARN = "ccs_minimize_roundtrips always defaults to true in Cross Project Search context."
19+
+ " Setting it explicitly has no effect irrespective of the value specified and is ignored.";
20+
21+
/**
22+
* For CPS, we do not necessarily want to use the MRT value that the user has provided.
23+
* Instead, we'd want to ignore it and default searches to `true` whilst issuing a warning
24+
* via the headers.
25+
* @param crossProjectEnabled If running in Cross Project Search environment.
26+
* @param request Rest request that we're parsing.
27+
* @return A boolean that determines if round trips should be minimised for this search request.
28+
*/
29+
public static boolean parseCcsMinimizeRoundtrips(Optional<Boolean> crossProjectEnabled, RestRequest request) {
30+
if (crossProjectEnabled.orElse(false)) {
31+
if (request.hasParam("ccs_minimize_roundtrips")) {
32+
request.param("ccs_minimize_roundtrips");
33+
HeaderWarning.addWarning(MRT_SET_IN_CPS_WARN);
34+
return true;
35+
}
36+
37+
// MRT was not provided; default to true.
38+
return true;
39+
} else {
40+
// This is not a CPS request; use the value the user has provided.
41+
return request.paramAsBoolean("ccs_minimize_roundtrips", true);
42+
}
43+
}
44+
}

‎server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.Collections;
4242
import java.util.List;
4343
import java.util.Map;
44+
import java.util.Optional;
4445

4546
import static java.util.Collections.singletonList;
4647
import static org.elasticsearch.search.RandomSearchRequestGenerator.randomSearchRequest;
@@ -98,7 +99,13 @@ public void testFailWithUnknownKey() {
9899
).build();
99100
IllegalArgumentException ex = expectThrows(
100101
IllegalArgumentException.class,
101-
() -> RestMultiSearchAction.parseRequest(restRequest, true, new UsageService().getSearchUsageHolder(), nf -> false, false)
102+
() -> RestMultiSearchAction.parseRequest(
103+
restRequest,
104+
true,
105+
new UsageService().getSearchUsageHolder(),
106+
nf -> false,
107+
Optional.empty()
108+
)
102109
);
103110
assertEquals("key [unknown_key] is not supported in the metadata section", ex.getMessage());
104111
}
@@ -117,7 +124,7 @@ public void testSimpleAddWithCarriageReturn() throws Exception {
117124
true,
118125
new UsageService().getSearchUsageHolder(),
119126
nf -> false,
120-
false
127+
Optional.empty()
121128
);
122129
assertThat(request.requests().size(), equalTo(1));
123130
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
@@ -141,7 +148,7 @@ public void testDefaultIndicesOptions() throws IOException {
141148
true,
142149
new UsageService().getSearchUsageHolder(),
143150
nf -> false,
144-
false
151+
Optional.empty()
145152
);
146153
assertThat(request.requests().size(), equalTo(1));
147154
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
@@ -251,7 +258,13 @@ public void testMsearchTerminatedByNewline() throws Exception {
251258
).build();
252259
IllegalArgumentException expectThrows = expectThrows(
253260
IllegalArgumentException.class,
254-
() -> RestMultiSearchAction.parseRequest(restRequest, true, new UsageService().getSearchUsageHolder(), nf -> false, false)
261+
() -> RestMultiSearchAction.parseRequest(
262+
restRequest,
263+
true,
264+
new UsageService().getSearchUsageHolder(),
265+
nf -> false,
266+
Optional.empty()
267+
)
255268
);
256269
assertEquals("The msearch request must be terminated by a newline [\n]", expectThrows.getMessage());
257270

@@ -265,7 +278,7 @@ public void testMsearchTerminatedByNewline() throws Exception {
265278
true,
266279
new UsageService().getSearchUsageHolder(),
267280
nf -> false,
268-
false
281+
Optional.empty()
269282
);
270283
assertEquals(3, msearchRequest.requests().size());
271284
}
@@ -286,7 +299,7 @@ private MultiSearchRequest parseMultiSearchRequest(RestRequest restRequest) thro
286299
new SearchSourceBuilder().parseXContent(parser, false, new UsageService().getSearchUsageHolder(), nf -> false)
287300
);
288301
request.add(searchRequest);
289-
}, false);
302+
}, Optional.empty());
290303
return request;
291304
}
292305

@@ -343,7 +356,8 @@ public void testMultiLineSerialization() throws IOException {
343356
null,
344357
null,
345358
null,
346-
true
359+
true,
360+
Optional.empty()
347361
);
348362
assertEquals(originalRequest, parsedRequest);
349363
}

0 commit comments

Comments
 (0)