Skip to content

Commit e5537ab

Browse files
javannajavanna
committed
High-level client: fix clusterAlias parsing in SearchHit (elastic#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.
1 parent 0951b95 commit e5537ab

File tree

8 files changed

+111
-96
lines changed

8 files changed

+111
-96
lines changed

‎server/src/main/java/org/elasticsearch/index/get/GetResult.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.index.get;
2121

2222
import org.elasticsearch.ElasticsearchParseException;
23+
import org.elasticsearch.common.Strings;
2324
import org.elasticsearch.common.bytes.BytesReference;
2425
import org.elasticsearch.common.compress.CompressorFactory;
2526
import org.elasticsearch.common.document.DocumentField;
@@ -226,10 +227,8 @@ public XContentBuilder toXContentEmbedded(XContentBuilder builder, Params params
226227
}
227228
}
228229
}
229-
230230
for (DocumentField field : metaFields) {
231-
Object value = field.getValue();
232-
builder.field(field.getName(), value);
231+
builder.field(field.getName(), field.<Object>getValue());
233232
}
234233

235234
builder.field(FOUND, exists);
@@ -399,7 +398,12 @@ public boolean equals(Object o) {
399398

400399
@Override
401400
public int hashCode() {
402-
return Objects.hash(index, type, id, version, exists, fields, sourceAsMap());
401+
return Objects.hash(version, exists, index, type, id, fields, sourceAsMap());
402+
}
403+
404+
@Override
405+
public String toString() {
406+
return Strings.toString(this, true, true);
403407
}
404408
}
405409

‎server/src/main/java/org/elasticsearch/search/SearchHit.java‎

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@
1919

2020
package org.elasticsearch.search;
2121

22+
import java.io.IOException;
23+
import java.util.ArrayList;
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.HashMap;
27+
import java.util.Iterator;
28+
import java.util.List;
29+
import java.util.Map;
30+
import java.util.Objects;
31+
2232
import org.apache.lucene.search.Explanation;
2333
import org.elasticsearch.ElasticsearchParseException;
2434
import org.elasticsearch.action.OriginalIndices;
@@ -51,16 +61,6 @@
5161
import org.elasticsearch.search.suggest.completion.CompletionSuggestion;
5262
import org.elasticsearch.transport.RemoteClusterAware;
5363

54-
import java.io.IOException;
55-
import java.util.ArrayList;
56-
import java.util.Arrays;
57-
import java.util.Collections;
58-
import java.util.HashMap;
59-
import java.util.Iterator;
60-
import java.util.List;
61-
import java.util.Map;
62-
import java.util.Objects;
63-
6464
import static java.util.Collections.emptyMap;
6565
import static java.util.Collections.singletonMap;
6666
import static java.util.Collections.unmodifiableMap;
@@ -107,6 +107,9 @@ public final class SearchHit implements Streamable, ToXContentObject, Iterable<D
107107
@Nullable
108108
private SearchShardTarget shard;
109109

110+
//These two fields normally get set when setting the shard target, so they hold the same values as the target thus don't get
111+
//serialized over the wire. When parsing hits back from xcontent though, in most of the cases (whenever explanation is disabled)
112+
//we can't rebuild the shard target object so we need to set these manually for users retrieval.
110113
private transient String index;
111114
private transient String clusterAlias;
112115

@@ -546,7 +549,26 @@ public static SearchHit createFromMap(Map<String, Object> values) {
546549
Map<String, DocumentField> fields = get(Fields.FIELDS, values, Collections.emptyMap());
547550

548551
SearchHit searchHit = new SearchHit(-1, id, type, nestedIdentity, fields);
549-
searchHit.index = get(Fields._INDEX, values, null);
552+
String index = get(Fields._INDEX, values, null);
553+
String clusterAlias = null;
554+
if (index != null) {
555+
int indexOf = index.indexOf(RemoteClusterAware.REMOTE_CLUSTER_INDEX_SEPARATOR);
556+
if (indexOf > 0) {
557+
clusterAlias = index.substring(0, indexOf);
558+
index = index.substring(indexOf + 1);
559+
}
560+
}
561+
ShardId shardId = get(Fields._SHARD, values, null);
562+
String nodeId = get(Fields._NODE, values, null);
563+
if (shardId != null && nodeId != null) {
564+
assert shardId.getIndexName().equals(index);
565+
searchHit.shard(new SearchShardTarget(nodeId, shardId, clusterAlias, OriginalIndices.NONE));
566+
} else {
567+
//these fields get set anyways when setting the shard target,
568+
//but we set them explicitly when we don't have enough info to rebuild the shard target
569+
searchHit.index = index;
570+
searchHit.clusterAlias = clusterAlias;
571+
}
550572
searchHit.score(get(Fields._SCORE, values, DEFAULT_SCORE));
551573
searchHit.version(get(Fields._VERSION, values, -1L));
552574
searchHit.sortValues(get(Fields.SORT, values, SearchSortValues.EMPTY));
@@ -556,12 +578,7 @@ public static SearchHit createFromMap(Map<String, Object> values) {
556578
searchHit.setInnerHits(get(Fields.INNER_HITS, values, null));
557579
List<String> matchedQueries = get(Fields.MATCHED_QUERIES, values, null);
558580
if (matchedQueries != null) {
559-
searchHit.matchedQueries(matchedQueries.toArray(new String[matchedQueries.size()]));
560-
}
561-
ShardId shardId = get(Fields._SHARD, values, null);
562-
String nodeId = get(Fields._NODE, values, null);
563-
if (shardId != null && nodeId != null) {
564-
searchHit.shard(new SearchShardTarget(nodeId, shardId, null, OriginalIndices.NONE));
581+
searchHit.matchedQueries(matchedQueries.toArray(new String[0]));
565582
}
566583
return searchHit;
567584
}
@@ -598,15 +615,12 @@ private static void declareMetaDataFields(ObjectParser<Map<String, Object>, Void
598615
if (metadatafield.equals(Fields._ID) == false && metadatafield.equals(Fields._INDEX) == false
599616
&& metadatafield.equals(Fields._TYPE) == false) {
600617
parser.declareField((map, field) -> {
601-
@SuppressWarnings("unchecked")
602-
Map<String, DocumentField> fieldMap = (Map<String, DocumentField>) map.computeIfAbsent(Fields.FIELDS,
618+
@SuppressWarnings("unchecked")
619+
Map<String, DocumentField> fieldMap = (Map<String, DocumentField>) map.computeIfAbsent(Fields.FIELDS,
603620
v -> new HashMap<String, DocumentField>());
604-
fieldMap.put(field.getName(), field);
605-
}, (p, c) -> {
606-
List<Object> values = new ArrayList<>();
607-
values.add(parseFieldsValue(p));
608-
return new DocumentField(metadatafield, values);
609-
}, new ParseField(metadatafield), ValueType.VALUE);
621+
fieldMap.put(field.getName(), field);
622+
}, (p, c) -> new DocumentField(metadatafield, Collections.singletonList(parseFieldsValue(p))),
623+
new ParseField(metadatafield), ValueType.VALUE);
610624
}
611625
}
612626
}
@@ -829,13 +843,15 @@ public boolean equals(Object obj) {
829843
&& Arrays.equals(matchedQueries, other.matchedQueries)
830844
&& Objects.equals(explanation, other.explanation)
831845
&& Objects.equals(shard, other.shard)
832-
&& Objects.equals(innerHits, other.innerHits);
846+
&& Objects.equals(innerHits, other.innerHits)
847+
&& Objects.equals(index, other.index)
848+
&& Objects.equals(clusterAlias, other.clusterAlias);
833849
}
834850

835851
@Override
836852
public int hashCode() {
837853
return Objects.hash(id, type, nestedIdentity, version, source, fields, getHighlightFields(), Arrays.hashCode(matchedQueries),
838-
explanation, shard, innerHits);
854+
explanation, shard, innerHits, index, clusterAlias);
839855
}
840856

841857
/**
@@ -953,4 +969,9 @@ public int hashCode() {
953969
return Objects.hash(field, offset, child);
954970
}
955971
}
972+
973+
@Override
974+
public String toString() {
975+
return Strings.toString(this, true, true);
976+
}
956977
}

‎server/src/main/java/org/elasticsearch/search/SearchShardTarget.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.search;
2121

22+
import java.io.IOException;
23+
2224
import org.elasticsearch.Version;
2325
import org.elasticsearch.action.OriginalIndices;
2426
import org.elasticsearch.common.Nullable;
@@ -30,16 +32,14 @@
3032
import org.elasticsearch.index.shard.ShardId;
3133
import org.elasticsearch.transport.RemoteClusterAware;
3234

33-
import java.io.IOException;
34-
3535
/**
3636
* The target that the search request was executed on.
3737
*/
3838
public final class SearchShardTarget implements Writeable, Comparable<SearchShardTarget> {
3939

4040
private final Text nodeId;
4141
private final ShardId shardId;
42-
//original indices and cluster alias are only needed in the coordinating node throughout the search request execution.
42+
//original indices are only needed in the coordinating node throughout the search request execution.
4343
//no need to serialize them as part of SearchShardTarget.
4444
private final transient OriginalIndices originalIndices;
4545
private final String clusterAlias;

‎server/src/test/java/org/elasticsearch/index/get/DocumentFieldTests.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@
2626
import org.elasticsearch.common.xcontent.ToXContent;
2727
import org.elasticsearch.common.xcontent.XContentParser;
2828
import org.elasticsearch.common.xcontent.XContentType;
29-
import org.elasticsearch.index.mapper.ParentFieldMapper;
30-
import org.elasticsearch.index.mapper.RoutingFieldMapper;
31-
import org.elasticsearch.index.mapper.UidFieldMapper;
29+
import org.elasticsearch.index.mapper.IdFieldMapper;
30+
import org.elasticsearch.index.mapper.IndexFieldMapper;
31+
import org.elasticsearch.index.mapper.MapperService;
32+
import org.elasticsearch.index.mapper.TypeFieldMapper;
3233
import org.elasticsearch.test.ESTestCase;
3334
import org.elasticsearch.test.RandomObjects;
3435

@@ -100,14 +101,17 @@ private static DocumentField mutateDocumentField(DocumentField documentField) {
100101

101102
public static Tuple<DocumentField, DocumentField> randomDocumentField(XContentType xContentType) {
102103
if (randomBoolean()) {
103-
String fieldName = randomFrom(ParentFieldMapper.NAME, RoutingFieldMapper.NAME, UidFieldMapper.NAME);
104-
DocumentField documentField = new DocumentField(fieldName, Collections.singletonList(randomAlphaOfLengthBetween(3, 10)));
104+
String metaField = randomValueOtherThanMany(field -> field.equals(TypeFieldMapper.NAME)
105+
|| field.equals(IndexFieldMapper.NAME) || field.equals(IdFieldMapper.NAME),
106+
() -> randomFrom(MapperService.getAllMetaFields()));
107+
DocumentField documentField = new DocumentField(metaField, Collections.singletonList(randomAlphaOfLengthBetween(3, 10)));
105108
return Tuple.tuple(documentField, documentField);
109+
} else {
110+
String fieldName = randomAlphaOfLengthBetween(3, 10);
111+
Tuple<List<Object>, List<Object>> tuple = RandomObjects.randomStoredFieldValues(random(), xContentType);
112+
DocumentField input = new DocumentField(fieldName, tuple.v1());
113+
DocumentField expected = new DocumentField(fieldName, tuple.v2());
114+
return Tuple.tuple(input, expected);
106115
}
107-
String fieldName = randomAlphaOfLengthBetween(3, 10);
108-
Tuple<List<Object>, List<Object>> tuple = RandomObjects.randomStoredFieldValues(random(), xContentType);
109-
DocumentField input = new DocumentField(fieldName, tuple.v1());
110-
DocumentField expected = new DocumentField(fieldName, tuple.v2());
111-
return Tuple.tuple(input, expected);
112116
}
113117
}

‎server/src/test/java/org/elasticsearch/index/get/GetResultTests.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ public void testToAndFromXContentEmbedded() throws Exception {
9090
XContentType xContentType = randomFrom(XContentType.values());
9191
Tuple<GetResult, GetResult> tuple = randomGetResult(xContentType);
9292
GetResult getResult = tuple.v1();
93-
9493
// We don't expect to retrieve the index/type/id of the GetResult because they are not rendered
9594
// by the toXContentEmbedded method.
9695
GetResult expectedGetResult = new GetResult(null, null, null, -1,
@@ -106,7 +105,6 @@ public void testToAndFromXContentEmbedded() throws Exception {
106105
parsedEmbeddedGetResult = GetResult.fromXContentEmbedded(parser);
107106
assertNull(parser.nextToken());
108107
}
109-
110108
assertEquals(expectedGetResult, parsedEmbeddedGetResult);
111109
//print the parsed object out and test that the output is the same as the original output
112110
BytesReference finalBytes = toXContentEmbedded(parsedEmbeddedGetResult, xContentType, humanReadable);
@@ -203,16 +201,17 @@ public static Tuple<GetResult, GetResult> randomGetResult(XContentType xContentT
203201
return Tuple.tuple(getResult, expectedGetResult);
204202
}
205203

206-
private static Tuple<Map<String, DocumentField>,Map<String, DocumentField>> randomDocumentFields(XContentType xContentType) {
204+
public static Tuple<Map<String, DocumentField>,Map<String, DocumentField>> randomDocumentFields(XContentType xContentType) {
207205
int numFields = randomIntBetween(2, 10);
208206
Map<String, DocumentField> fields = new HashMap<>(numFields);
209207
Map<String, DocumentField> expectedFields = new HashMap<>(numFields);
210-
for (int i = 0; i < numFields; i++) {
208+
while (fields.size() < numFields) {
211209
Tuple<DocumentField, DocumentField> tuple = randomDocumentField(xContentType);
212210
DocumentField getField = tuple.v1();
213211
DocumentField expectedGetField = tuple.v2();
214-
fields.put(getField.getName(), getField);
215-
expectedFields.put(expectedGetField.getName(), expectedGetField);
212+
if (fields.putIfAbsent(getField.getName(), getField) == null) {
213+
assertNull(expectedFields.putIfAbsent(expectedGetField.getName(), expectedGetField));
214+
}
216215
}
217216
return Tuple.tuple(fields, expectedFields);
218217
}

‎server/src/test/java/org/elasticsearch/search/SearchHitTests.java‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,40 +19,38 @@
1919

2020
package org.elasticsearch.search;
2121

22+
import java.io.IOException;
23+
import java.io.InputStream;
24+
import java.util.ArrayList;
25+
import java.util.Collections;
26+
import java.util.HashMap;
27+
import java.util.List;
28+
import java.util.Map;
29+
import java.util.function.Predicate;
30+
2231
import org.apache.lucene.search.Explanation;
2332
import org.elasticsearch.action.OriginalIndices;
2433
import org.elasticsearch.common.Strings;
2534
import org.elasticsearch.common.bytes.BytesArray;
2635
import org.elasticsearch.common.bytes.BytesReference;
27-
import org.elasticsearch.common.collect.Tuple;
2836
import org.elasticsearch.common.document.DocumentField;
2937
import org.elasticsearch.common.io.stream.BytesStreamOutput;
3038
import org.elasticsearch.common.io.stream.InputStreamStreamInput;
3139
import org.elasticsearch.common.text.Text;
32-
import org.elasticsearch.common.util.set.Sets;
3340
import org.elasticsearch.common.xcontent.ToXContent;
3441
import org.elasticsearch.common.xcontent.XContentBuilder;
3542
import org.elasticsearch.common.xcontent.XContentParser;
3643
import org.elasticsearch.common.xcontent.XContentType;
3744
import org.elasticsearch.common.xcontent.json.JsonXContent;
3845
import org.elasticsearch.index.Index;
46+
import org.elasticsearch.index.get.GetResultTests;
3947
import org.elasticsearch.index.shard.ShardId;
4048
import org.elasticsearch.search.SearchHit.NestedIdentity;
4149
import org.elasticsearch.search.fetch.subphase.highlight.HighlightField;
4250
import org.elasticsearch.search.fetch.subphase.highlight.HighlightFieldTests;
4351
import org.elasticsearch.test.ESTestCase;
4452
import org.elasticsearch.test.RandomObjects;
4553

46-
import java.io.IOException;
47-
import java.io.InputStream;
48-
import java.util.ArrayList;
49-
import java.util.Collections;
50-
import java.util.HashMap;
51-
import java.util.List;
52-
import java.util.Map;
53-
import java.util.Set;
54-
import java.util.function.Predicate;
55-
5654
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
5755
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
5856
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
@@ -63,8 +61,6 @@
6361

6462
public class SearchHitTests extends ESTestCase {
6563

66-
private static Set<String> META_FIELDS = Sets.newHashSet("_uid", "_all", "_parent", "_routing", "_size", "_timestamp", "_ttl");
67-
6864
public static SearchHit createTestItem(boolean withOptionalInnerHits) {
6965
int internalId = randomInt();
7066
String uid = randomAlphaOfLength(10);
@@ -75,18 +71,7 @@ public static SearchHit createTestItem(boolean withOptionalInnerHits) {
7571
}
7672
Map<String, DocumentField> fields = new HashMap<>();
7773
if (randomBoolean()) {
78-
int size = randomIntBetween(0, 10);
79-
for (int i = 0; i < size; i++) {
80-
Tuple<List<Object>, List<Object>> values = RandomObjects.randomStoredFieldValues(random(),
81-
XContentType.JSON);
82-
if (randomBoolean()) {
83-
String metaField = randomFrom(META_FIELDS);
84-
fields.put(metaField, new DocumentField(metaField, values.v1()));
85-
} else {
86-
String fieldName = randomAlphaOfLengthBetween(5, 10);
87-
fields.put(fieldName, new DocumentField(fieldName, values.v1()));
88-
}
89-
}
74+
fields = GetResultTests.randomDocumentFields(XContentType.JSON).v1();
9075
}
9176
SearchHit hit = new SearchHit(internalId, uid, type, nestedIdentity, fields);
9277
if (frequently()) {
@@ -109,7 +94,8 @@ public static SearchHit createTestItem(boolean withOptionalInnerHits) {
10994
int size = randomIntBetween(0, 5);
11095
Map<String, HighlightField> highlightFields = new HashMap<>(size);
11196
for (int i = 0; i < size; i++) {
112-
highlightFields.put(randomAlphaOfLength(5), HighlightFieldTests.createTestItem());
97+
HighlightField testItem = HighlightFieldTests.createTestItem();
98+
highlightFields.put(testItem.getName(), testItem);
11399
}
114100
hit.highlightFields(highlightFields);
115101
}
@@ -133,9 +119,10 @@ public static SearchHit createTestItem(boolean withOptionalInnerHits) {
133119
hit.setInnerHits(innerHits);
134120
}
135121
if (randomBoolean()) {
122+
String index = randomAlphaOfLengthBetween(5, 10);
123+
String clusterAlias = randomBoolean() ? null : randomAlphaOfLengthBetween(5, 10);
136124
hit.shard(new SearchShardTarget(randomAlphaOfLengthBetween(5, 10),
137-
new ShardId(new Index(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)), randomInt()), null,
138-
OriginalIndices.NONE));
125+
new ShardId(new Index(index, randomAlphaOfLengthBetween(5, 10)), randomInt()), clusterAlias, OriginalIndices.NONE));
139126
}
140127
return hit;
141128
}

‎server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ public static SearchSortValues createTestItem() {
4646
List<Supplier<Object>> valueSuppliers = new ArrayList<>();
4747
// this should reflect all values that are allowed to go through the transport layer
4848
valueSuppliers.add(() -> null);
49-
valueSuppliers.add(() -> randomInt());
50-
valueSuppliers.add(() -> randomLong());
51-
valueSuppliers.add(() -> randomDouble());
52-
valueSuppliers.add(() -> randomFloat());
53-
valueSuppliers.add(() -> randomByte());
54-
valueSuppliers.add(() -> randomShort());
55-
valueSuppliers.add(() -> randomBoolean());
49+
valueSuppliers.add(ESTestCase::randomInt);
50+
valueSuppliers.add(ESTestCase::randomLong);
51+
valueSuppliers.add(ESTestCase::randomDouble);
52+
valueSuppliers.add(ESTestCase::randomFloat);
53+
valueSuppliers.add(ESTestCase::randomByte);
54+
valueSuppliers.add(ESTestCase::randomShort);
55+
valueSuppliers.add(ESTestCase::randomBoolean);
5656
valueSuppliers.add(() -> frequently() ? randomAlphaOfLengthBetween(1, 30) : randomRealisticUnicodeOfCodepointLength(30));
5757

5858
int size = randomIntBetween(1, 20);

0 commit comments

Comments
 (0)