Skip to content

Commit 106ae82

Browse files
committed
Make ShardSearchTarget optional when parsing ShardSearchFailure (#27078)
Turns out that `ShardSearchTarget` is nullable, hence its fields may not be printed out as part of `ShardSearchFailure#toXContent`, in which case `fromXContent` cannot parse it back. We would previously try to create the object with all of its fields set to null, but `Index` complains about it in the constructor. Also made sure that this code path is covered by our unit tests in `ShardSearchFailureTests`. Closes #27055
1 parent a55c37c commit 106ae82

File tree

3 files changed

+20
-13
lines changed

3 files changed

+20
-13
lines changed

core/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ public String reason() {
131131

132132
@Override
133133
public String toString() {
134-
return "shard [" + (shardTarget == null ? "_na" : shardTarget) + "], reason [" + reason + "], cause [" + (cause == null ? "_na" : ExceptionsHelper.stackTrace(cause)) + "]";
134+
return "shard [" + (shardTarget == null ? "_na" : shardTarget) + "], reason [" + reason + "], cause [" +
135+
(cause == null ? "_na" : ExceptionsHelper.stackTrace(cause)) + "]";
135136
}
136137

137138
public static ShardSearchFailure readShardSearchFailure(StreamInput in) throws IOException {
@@ -210,9 +211,12 @@ public static ShardSearchFailure fromXContent(XContentParser parser) throws IOEx
210211
parser.skipChildren();
211212
}
212213
}
213-
return new ShardSearchFailure(exception,
214-
new SearchShardTarget(nodeId,
215-
new ShardId(new Index(indexName, IndexMetaData.INDEX_UUID_NA_VALUE), shardId), null, OriginalIndices.NONE));
214+
SearchShardTarget searchShardTarget = null;
215+
if (nodeId != null) {
216+
searchShardTarget = new SearchShardTarget(nodeId,
217+
new ShardId(new Index(indexName, IndexMetaData.INDEX_UUID_NA_VALUE), shardId), null, OriginalIndices.NONE);
218+
}
219+
return new ShardSearchFailure(exception, searchShardTarget);
216220
}
217221

218222
@Override

core/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public void testFromXContentWithFailures() throws IOException {
175175
ShardSearchFailure parsedFailure = parsed.getShardFailures()[i];
176176
ShardSearchFailure originalFailure = failures[i];
177177
assertEquals(originalFailure.index(), parsedFailure.index());
178-
assertEquals(originalFailure.shard().getNodeId(), parsedFailure.shard().getNodeId());
178+
assertEquals(originalFailure.shard(), parsedFailure.shard());
179179
assertEquals(originalFailure.shardId(), parsedFailure.shardId());
180180
String originalMsg = originalFailure.getCause().getMessage();
181181
assertEquals(parsedFailure.getCause().getMessage(), "Elasticsearch exception [type=parsing_exception, reason=" +

core/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java

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

2222
import org.elasticsearch.action.OriginalIndices;
23+
import org.elasticsearch.cluster.metadata.IndexMetaData;
2324
import org.elasticsearch.common.ParsingException;
2425
import org.elasticsearch.common.bytes.BytesReference;
2526
import org.elasticsearch.common.xcontent.ToXContent;
@@ -40,12 +41,14 @@ public class ShardSearchFailureTests extends ESTestCase {
4041
public static ShardSearchFailure createTestItem() {
4142
String randomMessage = randomAlphaOfLengthBetween(3, 20);
4243
Exception ex = new ParsingException(0, 0, randomMessage , new IllegalArgumentException("some bad argument"));
43-
String nodeId = randomAlphaOfLengthBetween(5, 10);
44-
String indexName = randomAlphaOfLengthBetween(5, 10);
45-
String indexUuid = randomAlphaOfLengthBetween(5, 10);
46-
int shardId = randomInt();
47-
return new ShardSearchFailure(ex,
48-
new SearchShardTarget(nodeId, new ShardId(new Index(indexName, indexUuid), shardId), null, null));
44+
SearchShardTarget searchShardTarget = null;
45+
if (randomBoolean()) {
46+
String nodeId = randomAlphaOfLengthBetween(5, 10);
47+
String indexName = randomAlphaOfLengthBetween(5, 10);
48+
searchShardTarget = new SearchShardTarget(nodeId,
49+
new ShardId(new Index(indexName, IndexMetaData.INDEX_UUID_NA_VALUE), randomInt()), null, null);
50+
}
51+
return new ShardSearchFailure(ex, searchShardTarget);
4952
}
5053

5154
public void testFromXContent() throws IOException {
@@ -80,10 +83,10 @@ private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws
8083
assertNull(parser.nextToken());
8184
}
8285
assertEquals(response.index(), parsed.index());
83-
assertEquals(response.shard().getNodeId(), parsed.shard().getNodeId());
86+
assertEquals(response.shard(), parsed.shard());
8487
assertEquals(response.shardId(), parsed.shardId());
8588

86-
/**
89+
/*
8790
* we cannot compare the cause, because it will be wrapped in an outer
8891
* ElasticSearchException best effort: try to check that the original
8992
* message appears somewhere in the rendered xContent

0 commit comments

Comments
 (0)