Skip to content

Commit 9e4105e

Browse files
jimczijimczi
committed
Validate PIT on _msearch (#63167)
This change ensures that we validate point in times provided by individual search requests in _msearch. Relates #63132
1 parent 38ee2da commit 9e4105e

File tree

4 files changed

+64
-26
lines changed

4 files changed

+64
-26
lines changed

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.common.Strings;
2929
import org.elasticsearch.common.bytes.BytesReference;
3030
import org.elasticsearch.common.collect.Tuple;
31+
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
3132
import org.elasticsearch.common.logging.DeprecationLogger;
3233
import org.elasticsearch.common.settings.Settings;
3334
import org.elasticsearch.common.xcontent.XContent;
@@ -90,8 +91,7 @@ public String getName() {
9091

9192
@Override
9293
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
93-
MultiSearchRequest multiSearchRequest = parseRequest(request, allowExplicitIndex);
94-
94+
final MultiSearchRequest multiSearchRequest = parseRequest(request, client.getNamedWriteableRegistry(), allowExplicitIndex);
9595
// Emit a single deprecation message if any search request contains types.
9696
for (SearchRequest searchRequest : multiSearchRequest.requests()) {
9797
if (searchRequest.types().length > 0) {
@@ -108,7 +108,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
108108
/**
109109
* Parses a {@link RestRequest} body and returns a {@link MultiSearchRequest}
110110
*/
111-
public static MultiSearchRequest parseRequest(RestRequest restRequest, boolean allowExplicitIndex) throws IOException {
111+
public static MultiSearchRequest parseRequest(RestRequest restRequest,
112+
NamedWriteableRegistry namedWriteableRegistry,
113+
boolean allowExplicitIndex) throws IOException {
112114
MultiSearchRequest multiRequest = new MultiSearchRequest();
113115
IndicesOptions indicesOptions = IndicesOptions.fromRequest(restRequest, multiRequest.indicesOptions());
114116
multiRequest.indicesOptions(indicesOptions);
@@ -133,6 +135,13 @@ public static MultiSearchRequest parseRequest(RestRequest restRequest, boolean a
133135
parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex, (searchRequest, parser) -> {
134136
searchRequest.source(SearchSourceBuilder.fromXContent(parser, false));
135137
RestSearchAction.checkRestTotalHits(restRequest, searchRequest);
138+
if (searchRequest.pointInTimeBuilder() != null) {
139+
RestSearchAction.preparePointInTime(searchRequest, restRequest, namedWriteableRegistry);
140+
} else {
141+
searchRequest.setCcsMinimizeRoundtrips(
142+
restRequest.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
143+
);
144+
}
136145
multiRequest.add(searchRequest);
137146
});
138147
List<SearchRequest> requests = multiRequest.requests();

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public void testFailWithUnknownKey() {
106106
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
107107
.withContent(new BytesArray(requestContent), XContentType.JSON).build();
108108
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
109-
() -> RestMultiSearchAction.parseRequest(restRequest, true));
109+
() -> RestMultiSearchAction.parseRequest(restRequest, null, true));
110110
assertEquals("key [unknown_key] is not supported in the metadata section", ex.getMessage());
111111
}
112112

@@ -115,7 +115,7 @@ public void testSimpleAddWithCarriageReturn() throws Exception {
115115
"{\"query\" : {\"match_all\" :{}}}\r\n";
116116
FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
117117
.withContent(new BytesArray(requestContent), XContentType.JSON).build();
118-
MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, true);
118+
MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, null, true);
119119
assertThat(request.requests().size(), equalTo(1));
120120
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
121121
assertThat(request.requests().get(0).indicesOptions(),
@@ -130,7 +130,7 @@ public void testDefaultIndicesOptions() throws IOException {
130130
.withContent(new BytesArray(requestContent), XContentType.JSON)
131131
.withParams(Collections.singletonMap("ignore_unavailable", "true"))
132132
.build();
133-
MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, true);
133+
MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, null, true);
134134
assertThat(request.requests().size(), equalTo(1));
135135
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
136136
assertThat(request.requests().get(0).indicesOptions(),
@@ -265,13 +265,13 @@ public void testMsearchTerminatedByNewline() throws Exception {
265265
RestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
266266
.withContent(new BytesArray(mserchAction.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build();
267267
IllegalArgumentException expectThrows = expectThrows(IllegalArgumentException.class,
268-
() -> RestMultiSearchAction.parseRequest(restRequest, true));
268+
() -> RestMultiSearchAction.parseRequest(restRequest, null, true));
269269
assertEquals("The msearch request must be terminated by a newline [\n]", expectThrows.getMessage());
270270

271271
String mserchActionWithNewLine = mserchAction + "\n";
272272
RestRequest restRequestWithNewLine = new FakeRestRequest.Builder(xContentRegistry())
273273
.withContent(new BytesArray(mserchActionWithNewLine.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build();
274-
MultiSearchRequest msearchRequest = RestMultiSearchAction.parseRequest(restRequestWithNewLine, true);
274+
MultiSearchRequest msearchRequest = RestMultiSearchAction.parseRequest(restRequestWithNewLine, null, true);
275275
assertEquals(3, msearchRequest.requests().size());
276276
}
277277

‎x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_data_stream_resolvability.yml‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,6 @@
648648

649649
- do:
650650
search:
651-
rest_total_hits_as_int: true
652651
body:
653652
size: 1
654653
query:
@@ -659,15 +658,14 @@
659658
id: "$point_in_time_id"
660659
keep_alive: 1m
661660

662-
- match: {hits.total: 3 }
661+
- match: {hits.total.value: 3 }
663662
- length: {hits.hits: 1 }
664663
- match: {hits.hits.0._index: .ds-simple-data-stream1-000001 }
665664
- match: {hits.hits.0._id: "123" }
666665
- match: {hits.hits.0.sort: [22, 123] }
667666

668667
- do:
669668
search:
670-
rest_total_hits_as_int: true
671669
body:
672670
size: 1
673671
query:
@@ -678,15 +676,14 @@
678676
pit:
679677
id: "$point_in_time_id"
680678

681-
- match: {hits.total: 3}
679+
- match: {hits.total.value: 3}
682680
- length: {hits.hits: 1 }
683681
- match: {hits.hits.0._index: .ds-simple-data-stream1-000001 }
684682
- match: {hits.hits.0._id: "5" }
685683
- match: {hits.hits.0.sort: [18, 5] }
686684

687685
- do:
688686
search:
689-
rest_total_hits_as_int: true
690687
body:
691688
size: 1
692689
query:
@@ -698,15 +695,14 @@
698695
id: "$point_in_time_id"
699696
keep_alive: 1m
700697

701-
- match: {hits.total: 3}
698+
- match: {hits.total.value: 3}
702699
- length: {hits.hits: 1 }
703700
- match: {hits.hits.0._index: .ds-simple-data-stream1-000001 }
704701
- match: {hits.hits.0._id: "1" }
705702
- match: {hits.hits.0.sort: [18, 1] }
706703

707704
- do:
708705
search:
709-
rest_total_hits_as_int: true
710706
body:
711707
size: 1
712708
query:
@@ -718,7 +714,7 @@
718714
id: "$point_in_time_id"
719715
keep_alive: 1m
720716

721-
- match: {hits.total: 3}
717+
- match: {hits.total.value: 3}
722718
- length: {hits.hits: 0 }
723719

724720
- do:

‎x-pack/plugin/src/test/resources/rest-api-spec/test/search/point_in_time.yml‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ setup:
4747

4848
- do:
4949
search:
50-
rest_total_hits_as_int: true
5150
body:
5251
size: 1
5352
query:
@@ -58,7 +57,7 @@ setup:
5857
id: "$point_in_time_id"
5958
keep_alive: 1m
6059

61-
- match: {hits.total: 3 }
60+
- match: {hits.total.value: 3 }
6261
- length: {hits.hits: 1 }
6362
- match: {hits.hits.0._index: test }
6463
- match: {hits.hits.0._id: "172" }
@@ -76,7 +75,6 @@ setup:
7675
# search with a point in time
7776
- do:
7877
search:
79-
rest_total_hits_as_int: true
8078
body:
8179
size: 1
8280
query:
@@ -87,15 +85,14 @@ setup:
8785
pit:
8886
id: "$point_in_time_id"
8987

90-
- match: {hits.total: 3 }
88+
- match: {hits.total.value: 3 }
9189
- length: {hits.hits: 1 }
9290
- match: {hits.hits.0._index: test }
9391
- match: {hits.hits.0._id: "42" }
9492
- match: {hits.hits.0.sort: [18, 42] }
9593

9694
- do:
9795
search:
98-
rest_total_hits_as_int: true
9996
body:
10097
size: 1
10198
query:
@@ -106,15 +103,14 @@ setup:
106103
pit:
107104
id: "$point_in_time_id"
108105

109-
- match: {hits.total: 3}
106+
- match: {hits.total.value: 3}
110107
- length: {hits.hits: 1 }
111108
- match: {hits.hits.0._index: test }
112109
- match: {hits.hits.0._id: "1" }
113110
- match: {hits.hits.0.sort: [18, 1] }
114111

115112
- do:
116113
search:
117-
rest_total_hits_as_int: true
118114
body:
119115
size: 1
120116
query:
@@ -126,7 +122,7 @@ setup:
126122
id: "$point_in_time_id"
127123
keep_alive: 1m
128124

129-
- match: {hits.total: 3}
125+
- match: {hits.total.value: 3}
130126
- length: {hits.hits: 0 }
131127

132128
- do:
@@ -147,7 +143,6 @@ setup:
147143

148144
- do:
149145
search:
150-
rest_total_hits_as_int: true
151146
body:
152147
size: 2
153148
query:
@@ -158,7 +153,7 @@ setup:
158153
id: "$point_in_time_id"
159154
keep_alive: 1m
160155

161-
- match: {hits.total: 4 }
156+
- match: {hits.total.value: 4 }
162157
- length: {hits.hits: 2 }
163158
- match: {hits.hits.0._index: test }
164159
- match: {hits.hits.0._id: "172" }
@@ -169,3 +164,41 @@ setup:
169164
close_point_in_time:
170165
body:
171166
id: "$point_in_time_id"
167+
168+
---
169+
"msearch":
170+
- skip:
171+
version: " - 7.9.99"
172+
reason: "point in time is introduced in 7.10"
173+
- do:
174+
open_point_in_time:
175+
index: "t*"
176+
keep_alive: 5m
177+
- set: {id: point_in_time_id}
178+
179+
- do:
180+
msearch:
181+
body:
182+
- {}
183+
- { query: { match: { _index: test }}, pit: { id: "$point_in_time_id" }}
184+
- {}
185+
- { query: { match_all: {}}, pit: { id: "$point_in_time_id" }}
186+
187+
- match: { responses.0.hits.total.value: 3 }
188+
- match: { responses.0.hits.total.relation: eq }
189+
- match: { responses.1.hits.total.value: 4 }
190+
- match: { responses.1.hits.total.relation: eq }
191+
192+
- do:
193+
catch: bad_request
194+
msearch:
195+
body:
196+
- index: index
197+
- { query: { match: { foo: bar }}, pit: { id: "$point_in_time_id" }}
198+
- index: index2
199+
- { query: { match_all: {}}, pit: { id: "$point_in_time_id" }}
200+
201+
- do:
202+
close_point_in_time:
203+
body:
204+
id: "$point_in_time_id"

0 commit comments

Comments
 (0)