Skip to content

Commit

Permalink
Improve resiliency to formatting JSON in server (#48706)
Browse files Browse the repository at this point in the history 
Backport of #48553. Make a number of changes so that JSON in the server
directory is more resilient to automatic formatting. This covers:

   * Reformatting multiline JSON to embed whitespace in the strings
   * Add helper method `stripWhitespace()`, to strip whitespace from a JSON
     document using XContent methods. This is sometimes necessary where
     a test is comparing some machine-generated JSON with an expected
     value.
  • Loading branch information
@pugnascotia
pugnascotia authored Oct 31, 2019
1 parent eefa84b commit d96976e
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 124 deletions.
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.compress.Compressor;
Expand Down Expand Up @@ -161,6 +162,19 @@ public static String convertToJson(BytesReference bytes, boolean reformatJson, X
return convertToJson(bytes, reformatJson, false, xContentType);
}

/**
* Accepts a JSON string, parses it and prints it without pretty-printing it. This is useful
* where a piece of JSON is formatted for legibility, but needs to be stripped of unnecessary
* whitespace e.g. for comparison in a test.
*
* @param json the JSON to format
* @return reformatted JSON
* @throws IOException if the reformatting fails, e.g. because the JSON is not well-formed
*/
public static String stripWhitespace(String json) throws IOException {
return convertToJson(new BytesArray(json), true, XContentType.JSON);
}

public static String convertToJson(BytesReference bytes, boolean reformatJson, boolean prettyPrint, XContentType xContentType)
throws IOException {
Objects.requireNonNull(xContentType);
Expand Down
65 changes: 35 additions & 30 deletions server/src/test/java/org/elasticsearch/action/search/SearchPhaseExecutionExceptionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.shard.IndexShardClosedException;
Expand Down Expand Up @@ -56,36 +57,40 @@ public void testToXContent() throws IOException {
});

// Failures are grouped (by default)
assertEquals("{" +
"\"type\":\"search_phase_execution_exception\"," +
"\"reason\":\"all shards failed\"," +
"\"phase\":\"test\"," +
"\"grouped\":true," +
"\"failed_shards\":[" +
"{" +
"\"shard\":0," +
"\"index\":\"foo\"," +
"\"node\":\"node_1\"," +
"\"reason\":{" +
"\"type\":\"parsing_exception\"," +
"\"reason\":\"foobar\"," +
"\"line\":1," +
"\"col\":2" +
"}" +
"}," +
"{" +
"\"shard\":1," +
"\"index\":\"foo\"," +
"\"node\":\"node_2\"," +
"\"reason\":{" +
"\"type\":\"index_shard_closed_exception\"," +
"\"reason\":\"CurrentState[CLOSED] Closed\"," +
"\"index_uuid\":\"_na_\"," +
"\"shard\":\"1\"," +
"\"index\":\"foo\"" +
"}" +
"}" +
"]}", Strings.toString(exception));
final String expectedJson = XContentHelper.stripWhitespace(
"{"
+ " \"type\": \"search_phase_execution_exception\","
+ " \"reason\": \"all shards failed\","
+ " \"phase\": \"test\","
+ " \"grouped\": true,"
+ " \"failed_shards\": ["
+ " {"
+ " \"shard\": 0,"
+ " \"index\": \"foo\","
+ " \"node\": \"node_1\","
+ " \"reason\": {"
+ " \"type\": \"parsing_exception\","
+ " \"reason\": \"foobar\","
+ " \"line\": 1,"
+ " \"col\": 2"
+ " }"
+ " },"
+ " {"
+ " \"shard\": 1,"
+ " \"index\": \"foo\","
+ " \"node\": \"node_2\","
+ " \"reason\": {"
+ " \"type\": \"index_shard_closed_exception\","
+ " \"reason\": \"CurrentState[CLOSED] Closed\","
+ " \"index_uuid\": \"_na_\","
+ " \"shard\": \"1\","
+ " \"index\": \"foo\""
+ " }"
+ " }"
+ " ]"
+ "}"
);
assertEquals(expectedJson, Strings.toString(exception));
}

public void testToAndFromXContent() throws IOException {
Expand Down
25 changes: 13 additions & 12 deletions server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,19 @@ public void testScriptSortTypeIllegalArgument() {
}

public void testParseJson() throws IOException {
String scriptSort = "{\n" +
"\"_script\" : {\n" +
"\"type\" : \"number\",\n" +
"\"script\" : {\n" +
"\"source\": \"doc['field_name'].value * factor\",\n" +
"\"params\" : {\n" +
"\"factor\" : 1.1\n" +
"}\n" +
"},\n" +
"\"mode\" : \"max\",\n" +
"\"order\" : \"asc\"\n" +
"} }\n";
String scriptSort = "{"
+ " \"_script\": {"
+ " \"type\": \"number\","
+ " \"script\": {"
+ " \"source\": \"doc['field_name'].value * factor\","
+ " \"params\": {"
+ " \"factor\": 1.1"
+ " }"
+ " },"
+ " \"mode\": \"max\","
+ " \"order\": \"asc\""
+ " }"
+ "}";
try (XContentParser parser = createParser(JsonXContent.jsonXContent, scriptSort)) {
parser.nextToken();
parser.nextToken();
Expand Down
36 changes: 23 additions & 13 deletions server/src/test/java/org/elasticsearch/search/suggest/SuggestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.List;

import static java.util.Collections.emptyList;
import static org.elasticsearch.common.xcontent.XContentHelper.stripWhitespace;
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
Expand Down Expand Up @@ -131,19 +132,28 @@ public void testToXContent() throws IOException {
Suggest suggest = new Suggest(Collections.singletonList(suggestion));
BytesReference xContent = toXContent(suggest, XContentType.JSON, randomBoolean());
assertEquals(
"{\"suggest\":"
+ "{\"suggestionName\":"
+ "[{\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{\"text\":\"someText\","
+ "\"highlighted\":\"somethingHighlighted\","
+ "\"score\":1.3,"
+ "\"collate_match\":true}]"
+ "}]"
+ "}"
+"}",
xContent.utf8ToString());
stripWhitespace(
"{"
+ " \"suggest\": {"
+ " \"suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"highlighted\": \"somethingHighlighted\","
+ " \"score\": 1.3,"
+ " \"collate_match\": true"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ " }"
+ "}"
),
xContent.utf8ToString());
}

public void testFilter() throws Exception {
Expand Down
15 changes: 9 additions & 6 deletions server/src/test/java/org/elasticsearch/search/suggest/SuggestionOptionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,14 @@ private void doTestFromXContent(boolean addRandomFields) throws IOException {
public void testToXContent() throws IOException {
Option option = new PhraseSuggestion.Entry.Option(new Text("someText"), new Text("somethingHighlighted"), 1.3f, true);
BytesReference xContent = toXContent(option, XContentType.JSON, randomBoolean());
assertEquals("{\"text\":\"someText\","
+ "\"highlighted\":\"somethingHighlighted\","
+ "\"score\":1.3,"
+ "\"collate_match\":true"
+ "}"
, xContent.utf8ToString());
assertEquals(
("{"
+ " \"text\": \"someText\","
+ " \"highlighted\": \"somethingHighlighted\","
+ " \"score\": 1.3,"
+ " \"collate_match\": true"
+ "}").replaceAll("\\s+", ""),
xContent.utf8ToString()
);
}
}
150 changes: 95 additions & 55 deletions server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,23 @@ public void testFromXContentWithoutTypeParam() throws IOException {

public void testUnknownSuggestionTypeThrows() throws IOException {
XContent xContent = JsonXContent.jsonXContent;
String suggestionString =
"{\"unknownType#suggestionName\":"
+ "[{\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{\"text\":\"someText\","
+ "\"highlighted\":\"somethingHighlighted\","
+ "\"score\":1.3,"
+ "\"collate_match\":true}]"
+ "}]"
+ "}";
String suggestionString = ("{"
+ " \"unknownType#suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"highlighted\": \"somethingHighlighted\","
+ " \"score\": 1.3,"
+ " \"collate_match\": true"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ "}").replaceAll("\\s+", "");
try (XContentParser parser = xContent.createParser(xContentRegistry(),
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, suggestionString)) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
Expand All @@ -195,18 +201,25 @@ public void testToXContent() throws IOException {
PhraseSuggestion suggestion = new PhraseSuggestion("suggestionName", 5);
suggestion.addTerm(entry);
BytesReference xContent = toXContent(suggestion, XContentType.JSON, params, randomBoolean());
assertEquals(
"{\"phrase#suggestionName\":[{"
+ "\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{"
+ "\"text\":\"someText\","
+ "\"highlighted\":\"somethingHighlighted\","
+ "\"score\":1.3,"
+ "\"collate_match\":true}]"
+ "}]"
+ "}", xContent.utf8ToString());
assertEquals(("{"
+ " \"phrase#suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"highlighted\": \"somethingHighlighted\","
+ " \"score\": 1.3,"
+ " \"collate_match\": true"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ "}").replaceAll("\\s+", ""),
xContent.utf8ToString()
);
}
{
PhraseSuggestion.Entry.Option option = new PhraseSuggestion.Entry.Option(new Text("someText"), new Text("somethingHighlighted"),
Expand All @@ -216,18 +229,25 @@ public void testToXContent() throws IOException {
PhraseSuggestion suggestion = new PhraseSuggestion("suggestionName", 5);
suggestion.addTerm(entry);
BytesReference xContent = toXContent(suggestion, XContentType.JSON, params, randomBoolean());
assertEquals(
"{\"phrase#suggestionName\":[{"
+ "\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{"
+ "\"text\":\"someText\","
+ "\"highlighted\":\"somethingHighlighted\","
+ "\"score\":1.3,"
+ "\"collate_match\":true}]"
+ "}]"
+ "}", xContent.utf8ToString());
assertEquals(("{"
+ " \"phrase#suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"highlighted\": \"somethingHighlighted\","
+ " \"score\": 1.3,"
+ " \"collate_match\": true"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ "}").replaceAll("\\s+", ""),
xContent.utf8ToString()
);
}
{
TermSuggestion.Entry.Option option = new TermSuggestion.Entry.Option(new Text("someText"), 10, 1.3f);
Expand All @@ -237,16 +257,24 @@ public void testToXContent() throws IOException {
suggestion.addTerm(entry);
BytesReference xContent = toXContent(suggestion, XContentType.JSON, params, randomBoolean());
assertEquals(
"{\"term#suggestionName\":[{"
+ "\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{"
+ "\"text\":\"someText\","
+ "\"score\":1.3,"
+ "\"freq\":10}]"
+ "}]"
+ "}", xContent.utf8ToString());
("{"
+ " \"term#suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"score\": 1.3,"
+ " \"freq\": 10"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ "}").replaceAll("\\s+", ""),
xContent.utf8ToString()
);
}
{
Map<String, Set<String>> contexts = Collections.singletonMap("key", Collections.singleton("value"));
Expand All @@ -257,16 +285,28 @@ public void testToXContent() throws IOException {
suggestion.addTerm(entry);
BytesReference xContent = toXContent(suggestion, XContentType.JSON, params, randomBoolean());
assertEquals(
"{\"completion#suggestionName\":[{"
+ "\"text\":\"entryText\","
+ "\"offset\":42,"
+ "\"length\":313,"
+ "\"options\":[{"
+ "\"text\":\"someText\","
+ "\"score\":1.3,"
+ "\"contexts\":{\"key\":[\"value\"]}"
+ "}]"
+ "}]}", xContent.utf8ToString());
("{"
+ " \"completion#suggestionName\": ["
+ " {"
+ " \"text\": \"entryText\","
+ " \"offset\": 42,"
+ " \"length\": 313,"
+ " \"options\": ["
+ " {"
+ " \"text\": \"someText\","
+ " \"score\": 1.3,"
+ " \"contexts\": {"
+ " \"key\": ["
+ " \"value\""
+ " ]"
+ " }"
+ " }"
+ " ]"
+ " }"
+ " ]"
+ "}").replaceAll("\\s+", ""),
xContent.utf8ToString()
);
}
}
}
Loading

0 comments on commit d96976e

Please sign in to comment.