Skip to content

Commit 55e98ed

Browse files
gaobinlonggaobinlong
authored
Fix multi-search with template doesn't return status code (#16265)
* Fix multi-search with template doesn't return status code Signed-off-by: Gao Binlong <gbinlong@amazon.com> * Modify changelog Signed-off-by: Gao Binlong <gbinlong@amazon.com> --------- Signed-off-by: Gao Binlong <gbinlong@amazon.com>
1 parent 2166b44 commit 55e98ed

File tree

6 files changed

+136
-1
lines changed

6 files changed

+136
-1
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
7373
- Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158))
7474
- Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194))
7575
- Fix protobuf-java leak through client library dependencies ([#16254](https://github.com/opensearch-project/OpenSearch/pull/16254))
76+
- Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265))
7677

7778
### Security
7879

‎modules/lang-mustache/src/main/java/org/opensearch/script/mustache/MultiSearchTemplateResponse.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
package org.opensearch.script.mustache;
3434

35+
import org.opensearch.ExceptionsHelper;
3536
import org.opensearch.OpenSearchException;
3637
import org.opensearch.action.search.MultiSearchResponse;
3738
import org.opensearch.common.Nullable;
@@ -167,6 +168,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
167168
if (item.isFailure()) {
168169
builder.startObject();
169170
OpenSearchException.generateFailureXContent(builder, params, item.getFailure(), true);
171+
builder.field(Fields.STATUS, ExceptionsHelper.status(item.getFailure()).getStatus());
170172
builder.endObject();
171173
} else {
172174
item.getResponse().toXContent(builder, params);
@@ -179,6 +181,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
179181

180182
static final class Fields {
181183
static final String RESPONSES = "responses";
184+
static final String STATUS = "status";
182185
}
183186

184187
public static MultiSearchTemplateResponse fromXContext(XContentParser parser) {

‎modules/lang-mustache/src/main/java/org/opensearch/script/mustache/SearchTemplateResponse.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,10 @@ public static SearchTemplateResponse fromXContent(XContentParser parser) throws
120120
@Override
121121
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
122122
if (hasResponse()) {
123-
response.toXContent(builder, params);
123+
builder.startObject();
124+
response.innerToXContent(builder, params);
125+
builder.field(MultiSearchTemplateResponse.Fields.STATUS, response.status().getStatus());
126+
builder.endObject();
124127
} else {
125128
builder.startObject();
126129
// we can assume the template is always json as we convert it before compiling it

‎modules/lang-mustache/src/test/java/org/opensearch/script/mustache/MultiSearchTemplateResponseTests.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,12 @@
3434
import org.opensearch.OpenSearchException;
3535
import org.opensearch.action.search.SearchResponse;
3636
import org.opensearch.action.search.ShardSearchFailure;
37+
import org.opensearch.common.xcontent.XContentType;
3738
import org.opensearch.core.common.Strings;
39+
import org.opensearch.core.common.bytes.BytesReference;
40+
import org.opensearch.core.xcontent.MediaTypeRegistry;
3841
import org.opensearch.core.xcontent.ToXContent;
42+
import org.opensearch.core.xcontent.XContentBuilder;
3943
import org.opensearch.core.xcontent.XContentParser;
4044
import org.opensearch.search.internal.InternalSearchResponse;
4145
import org.opensearch.test.AbstractXContentTestCase;
@@ -44,6 +48,7 @@
4448
import java.util.function.Predicate;
4549
import java.util.function.Supplier;
4650

51+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertToXContentEquivalent;
4752
import static org.hamcrest.Matchers.containsString;
4853
import static org.hamcrest.Matchers.equalTo;
4954
import static org.hamcrest.Matchers.nullValue;
@@ -177,4 +182,78 @@ public void testFromXContentWithFailures() throws IOException {
177182
ToXContent.EMPTY_PARAMS
178183
);
179184
}
185+
186+
public void testToXContentWithFailures() throws Exception {
187+
long overallTookInMillis = randomNonNegativeLong();
188+
MultiSearchTemplateResponse.Item[] items = new MultiSearchTemplateResponse.Item[2];
189+
190+
long tookInMillis = 1L;
191+
int totalShards = 2;
192+
int successfulShards = 2;
193+
int skippedShards = 0;
194+
InternalSearchResponse internalSearchResponse = InternalSearchResponse.empty();
195+
SearchTemplateResponse searchTemplateResponse = new SearchTemplateResponse();
196+
SearchResponse searchResponse = new SearchResponse(
197+
internalSearchResponse,
198+
null,
199+
totalShards,
200+
successfulShards,
201+
skippedShards,
202+
tookInMillis,
203+
ShardSearchFailure.EMPTY_ARRAY,
204+
SearchResponse.Clusters.EMPTY
205+
);
206+
searchTemplateResponse.setResponse(searchResponse);
207+
items[0] = new MultiSearchTemplateResponse.Item(searchTemplateResponse, null);
208+
209+
items[1] = new MultiSearchTemplateResponse.Item(null, new IllegalArgumentException("Invalid argument"));
210+
211+
MultiSearchTemplateResponse response = new MultiSearchTemplateResponse(items, overallTookInMillis);
212+
213+
XContentType contentType = randomFrom(XContentType.values());
214+
XContentBuilder expectedResponse = MediaTypeRegistry.contentBuilder(contentType)
215+
.startObject()
216+
.field("took", overallTookInMillis)
217+
.startArray("responses")
218+
.startObject()
219+
.field("took", 1)
220+
.field("timed_out", false)
221+
.startObject("_shards")
222+
.field("total", 2)
223+
.field("successful", 2)
224+
.field("skipped", 0)
225+
.field("failed", 0)
226+
.endObject()
227+
.startObject("hits")
228+
.startObject("total")
229+
.field("value", 0)
230+
.field("relation", "eq")
231+
.endObject()
232+
.field("max_score", 0.0F)
233+
.startArray("hits")
234+
.endArray()
235+
.endObject()
236+
.field("status", 200)
237+
.endObject()
238+
.startObject()
239+
.startObject("error")
240+
.field("type", "illegal_argument_exception")
241+
.field("reason", "Invalid argument")
242+
.startArray("root_cause")
243+
.startObject()
244+
.field("type", "illegal_argument_exception")
245+
.field("reason", "Invalid argument")
246+
.endObject()
247+
.endArray()
248+
.endObject()
249+
.field("status", 400)
250+
.endObject()
251+
.endArray()
252+
.endObject();
253+
254+
XContentBuilder actualResponse = MediaTypeRegistry.contentBuilder(contentType);
255+
response.toXContent(actualResponse, ToXContent.EMPTY_PARAMS);
256+
257+
assertToXContentEquivalent(BytesReference.bytes(expectedResponse), BytesReference.bytes(actualResponse), contentType);
258+
}
180259
}

‎modules/lang-mustache/src/test/java/org/opensearch/script/mustache/SearchTemplateResponseTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ public void testSearchResponseToXContent() throws IOException {
234234
.endObject()
235235
.endArray()
236236
.endObject()
237+
.field("status", 200)
237238
.endObject();
238239

239240
XContentBuilder actualResponse = MediaTypeRegistry.contentBuilder(contentType);

‎modules/lang-mustache/src/yamlRestTest/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,51 @@ setup:
205205
- match: { responses.1.hits.total.relation: eq }
206206
- match: { responses.2.hits.total.value: 1 }
207207
- match: { responses.1.hits.total.relation: eq }
208+
209+
---
210+
"Test multi-search template returns status code":
211+
- skip:
212+
version: " - 2.17.99"
213+
reason: Fixed in 2.18.0.
214+
- do:
215+
msearch_template:
216+
rest_total_hits_as_int: true
217+
body:
218+
# Search 0 is OK
219+
- index: index_*
220+
- source: '{"query": {"match": {"foo": "{{value}}"} } }'
221+
params:
222+
value: "foo"
223+
# Search 1 has an unclosed JSON template
224+
- index: index_*
225+
- source: '{"query": {"match": {'
226+
params:
227+
field: "foo"
228+
value: "bar"
229+
# Search 2 is OK
230+
- index: _all
231+
- source:
232+
query:
233+
match: {foo: "{{text}}"}
234+
params:
235+
text: "baz"
236+
# Search 3 has an unknown query type
237+
- index: index_*
238+
- source: '{"query": {"{{query_type}}": {} }' # Unknown query type
239+
params:
240+
query_type: "unknown"
241+
# Search 4 has an unsupported track_total_hits
242+
- index: index_*
243+
- source: '{"query": {"match_all": {} }, "track_total_hits": "{{trackTotalHits}}" }'
244+
params:
245+
trackTotalHits: 1
246+
# Search 5 has an unknown index
247+
- index: unknown_index
248+
- source: '{"query": {"match_all": {} } }'
249+
250+
- match: { responses.0.status: 200 }
251+
- match: { responses.1.status: 400 }
252+
- match: { responses.2.status: 200 }
253+
- match: { responses.3.status: 400 }
254+
- match: { responses.4.status: 400 }
255+
- match: { responses.5.status: 404 }

0 commit comments

Comments
 (0)