Browse Source

Get Async Search: omit _clusters section when empty (#53907)

The _clusters section is omitted by the search API whenever no remote clusters are searched. Async search should do the same, but Get Async Search returns a deserialized response, hence a weird `_clusters` section with all values set to `0` gets returned instead. In fact the recreated Clusters object is not the same object as the EMPTY constant, yet it has the same content.

This commit addresses this by changing the comparison in the `toXContent` method to not print out the section if the number of total clusters is `0`.
Luca Cavanna 5 years ago
parent
commit
c334b2045a

+ 2 - 4
server/src/main/java/org/elasticsearch/action/search/SearchResponse.java

@@ -413,9 +413,7 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
         }
 
         private Clusters(StreamInput in) throws IOException {
-            this.total = in.readVInt();
-            this.successful = in.readVInt();
-            this.skipped = in.readVInt();
+            this(in.readVInt(), in.readVInt(), in.readVInt());
         }
 
         @Override
@@ -427,7 +425,7 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            if (this != EMPTY) {
+            if (total > 0) {
                 builder.startObject(_CLUSTERS_FIELD.getPreferredName());
                 builder.field(TOTAL_FIELD.getPreferredName(), total);
                 builder.field(SUCCESSFUL_FIELD.getPreferredName(), successful);

+ 10 - 0
server/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
@@ -291,4 +292,13 @@ public class SearchResponseTests extends ESTestCase {
         assertEquals(searchResponse.getSkippedShards(), deserialized.getSkippedShards());
         assertEquals(searchResponse.getClusters(), deserialized.getClusters());
     }
+
+    public void testToXContentEmptyClusters() throws IOException {
+        SearchResponse searchResponse = new SearchResponse(InternalSearchResponse.empty(), null, 1, 1, 0, 1,
+            ShardSearchFailure.EMPTY_ARRAY, SearchResponse.Clusters.EMPTY);
+        SearchResponse deserialized = copyWriteable(searchResponse, namedWriteableRegistry, SearchResponse::new, Version.CURRENT);
+        XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
+        deserialized.getClusters().toXContent(builder, ToXContent.EMPTY_PARAMS);
+        assertEquals(0, Strings.toString(builder).length());
+    }
 }

+ 3 - 1
x-pack/plugin/src/test/resources/rest-api-spec/test/async_search/10_basic.yml

@@ -73,7 +73,6 @@
                 field: max
           sort: max
 
-  - set:    { id:                              id }
   - match:  { version:                          6 }
   - match:  { is_partial:                   false }
   - length: { response.hits.hits:               3 }
@@ -98,6 +97,7 @@
   - set:    { id:                                  id }
   - match:  { version:                              6 }
   - match:  { is_partial:                       false }
+  - is_false: response._clusters
   - length: { response.hits.hits:                   3 }
   - match:  { response.hits.hits.0._source.max:     1 }
   - match:  { response.aggregations.max#max.value:  3.0 }
@@ -108,6 +108,7 @@
 
   - match:  { version:                            6 }
   - match:  { is_partial:                     false }
+  - is_false: response._clusters
   - length: { response.hits.hits:                 3 }
   - match:  { response.hits.hits.0._source.max:   1 }
   - match:  { response.aggregations.max.value:    3.0 }
@@ -120,6 +121,7 @@
 
   - match:  { version:                              6 }
   - match:  { is_partial:                       false }
+  - is_false: response._clusters
   - length: { response.hits.hits:                   3 }
   - match:  { response.hits.hits.0._source.max:     1 }
   - match:  { response.aggregations.max#max.value:  3.0 }