High-level client: fix clusterAlias parsing in SearchHit (#32465)

When using cross-cluster search through the high-level REST client, the cluster alias from each search hit was not parsed correctly. It would be part of the index field initially, but overridden just a few lines later once setting the shard target (in case we have enough info to build it from the response). In any case, getClusterAlias returns `null` which is a bug. With this change we rather parse back clusterAliases from the index name, set its corresponding field and properly handle the two possible cases depending on whether we can or cannot build the shard target object.
elastic · Jul 31, 2018 · a3b2729 · a3b2729
1 parent 8b57e2e
commit a3b2729
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 24 deletions.
diff --git a/server/src/main/java/org/elasticsearch/search/SearchHit.java b/server/src/main/java/org/elasticsearch/search/SearchHit.java
@@ -19,6 +19,16 @@
 
 package org.elasticsearch.search;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
 import org.apache.lucene.search.Explanation;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.action.OriginalIndices;
@@ -51,16 +61,6 @@
 import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.transport.RemoteClusterAware;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static java.util.Collections.unmodifiableMap;
@@ -107,6 +107,9 @@ public final class SearchHit implements Streamable, ToXContentObject, Iterable<D
     @Nullable
     private SearchShardTarget shard;
 
+    //These two fields normally get set when setting the shard target, so they hold the same values as the target thus don't get
+    //serialized over the wire. When parsing hits back from xcontent though, in most of the cases (whenever explanation is disabled)
+    //we can't rebuild the shard target object so we need to set these manually for users retrieval.
     private transient String index;
     private transient String clusterAlias;
 
@@ -551,7 +554,26 @@ public static SearchHit createFromMap(Map<String, Object> values) {
         Map<String, DocumentField> fields = get(Fields.FIELDS, values, Collections.emptyMap());
 
         SearchHit searchHit = new SearchHit(-1, id, type, nestedIdentity, fields);
-        searchHit.index = get(Fields._INDEX, values, null);
+        String index = get(Fields._INDEX, values, null);
+        String clusterAlias = null;
+        if (index != null) {
+            int indexOf = index.indexOf(RemoteClusterAware.REMOTE_CLUSTER_INDEX_SEPARATOR);
+            if (indexOf > 0) {
+                clusterAlias = index.substring(0, indexOf);
+                index = index.substring(indexOf + 1);
+            }
+        }
+        ShardId shardId = get(Fields._SHARD, values, null);
+        String nodeId = get(Fields._NODE, values, null);
+        if (shardId != null && nodeId != null) {
+            assert shardId.getIndexName().equals(index);
+            searchHit.shard(new SearchShardTarget(nodeId, shardId, clusterAlias, OriginalIndices.NONE));
+        } else {
+            //these fields get set anyways when setting the shard target,
+            //but we set them explicitly when we don't have enough info to rebuild the shard target
+            searchHit.index = index;
+            searchHit.clusterAlias = clusterAlias;
+        }
         searchHit.score(get(Fields._SCORE, values, DEFAULT_SCORE));
         searchHit.version(get(Fields._VERSION, values, -1L));
         searchHit.sortValues(get(Fields.SORT, values, SearchSortValues.EMPTY));
@@ -561,12 +583,7 @@ public static SearchHit createFromMap(Map<String, Object> values) {
         searchHit.setInnerHits(get(Fields.INNER_HITS, values, null));
         List<String> matchedQueries = get(Fields.MATCHED_QUERIES, values, null);
         if (matchedQueries != null) {
-            searchHit.matchedQueries(matchedQueries.toArray(new String[matchedQueries.size()]));
-        }
-        ShardId shardId = get(Fields._SHARD, values, null);
-        String nodeId = get(Fields._NODE, values, null);
-        if (shardId != null && nodeId != null) {
-            searchHit.shard(new SearchShardTarget(nodeId, shardId, null, OriginalIndices.NONE));
+            searchHit.matchedQueries(matchedQueries.toArray(new String[0]));
         }
         return searchHit;
     }
@@ -842,13 +859,15 @@ public boolean equals(Object obj) {
                 && Arrays.equals(matchedQueries, other.matchedQueries)
                 && Objects.equals(explanation, other.explanation)
                 && Objects.equals(shard, other.shard)
-                && Objects.equals(innerHits, other.innerHits);
+                && Objects.equals(innerHits, other.innerHits)
+                && Objects.equals(index, other.index)
+                && Objects.equals(clusterAlias, other.clusterAlias);
     }
 
     @Override
     public int hashCode() {
         return Objects.hash(id, type, nestedIdentity, version, source, fields, getHighlightFields(), Arrays.hashCode(matchedQueries),
-                explanation, shard, innerHits);
+                explanation, shard, innerHits, index, clusterAlias);
     }
 
     /**

diff --git a/server/src/main/java/org/elasticsearch/search/SearchShardTarget.java b/server/src/main/java/org/elasticsearch/search/SearchShardTarget.java
@@ -19,6 +19,8 @@
 
 package org.elasticsearch.search;
 
+import java.io.IOException;
+
 import org.elasticsearch.Version;
 import org.elasticsearch.action.OriginalIndices;
 import org.elasticsearch.common.Nullable;
@@ -30,16 +32,14 @@
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.transport.RemoteClusterAware;
 
-import java.io.IOException;
-
 /**
  * The target that the search request was executed on.
  */
 public final class SearchShardTarget implements Writeable, Comparable<SearchShardTarget> {
 
     private final Text nodeId;
     private final ShardId shardId;
-    //original indices and cluster alias are only needed in the coordinating node throughout the search request execution.
+    //original indices are only needed in the coordinating node throughout the search request execution.
     //no need to serialize them as part of SearchShardTarget.
     private final transient OriginalIndices originalIndices;
     private final String clusterAlias;

diff --git a/server/src/test/java/org/elasticsearch/search/SearchHitTests.java b/server/src/test/java/org/elasticsearch/search/SearchHitTests.java
@@ -119,9 +119,10 @@ public static SearchHit createTestItem(boolean withOptionalInnerHits) {
             hit.setInnerHits(innerHits);
         }
         if (randomBoolean()) {
+            String index = randomAlphaOfLengthBetween(5, 10);
+            String clusterAlias = randomBoolean() ? null : randomAlphaOfLengthBetween(5, 10);
             hit.shard(new SearchShardTarget(randomAlphaOfLengthBetween(5, 10),
-                    new ShardId(new Index(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)), randomInt()), null,
-                    OriginalIndices.NONE));
+                new ShardId(new Index(index, randomAlphaOfLengthBetween(5, 10)), randomInt()), clusterAlias, OriginalIndices.NONE));
         }
         return hit;
     }