honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove XContentHelper#toString(ToXContent) in favour of Strings#toString(ToXContent) (#25866) 

These two methods do do the same thing. The subtle difference between the two is that the former prints out pretty printed content by default while the latter doesn't. There are way more usages of the latter throughout the codebase hence I kept that variant although I do think that it would be much better to print out prettified content by default from a `toString`. That breaks quite some tests so I didn't make that change yet.

Also XContentHelper#toString was outdated as it didn't check the ToXContent#isFragment method to decide whether a new anonymous object has to be created or not. It would simply fail with any ToXContentObject.
This commit is contained in:
Luca Cavanna 2017-07-26 16:00:59 +02:00 committed by GitHub
parent ff7c749c70
commit d8203f19fd
6 changed files with 62 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
core/src/main/java/org/elasticsearch/common/Strings.java
View File

@ -20,6 +20,7 @@
package org.elasticsearch.common;
import org.apache.lucene.util.BytesRefBuilder;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.FastStringReader;
@ -36,7 +37,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.StringTokenizer;
@ -792,11 +792,22 @@ public class Strings {
/**
* Return a {@link String} that is the json representation of the provided {@link ToXContent}.
* Wraps the output into an anonymous object.
* Wraps the output into an anonymous object if needed. The content is not pretty-printed
* nor human readable.
*/
public static String toString(ToXContent toXContent) {
return toString(toXContent, false, false);
}
/**
* Return a {@link String} that is the json representation of the provided {@link ToXContent}.
* Wraps the output into an anonymous object if needed. Allows to control whether the outputted
* json needs to be pretty printed and human readable.
*
*/
public static String toString(ToXContent toXContent, boolean pretty, boolean human) {
try {
XContentBuilder builder = JsonXContent.contentBuilder();
XContentBuilder builder = createBuilder(pretty, human);
if (toXContent.isFragment()) {
builder.startObject();
}
@ -806,10 +817,30 @@ public class Strings {
}
return builder.string();
} catch (IOException e) {
return "Error building toString out of XContent: " + ExceptionsHelper.stackTrace(e);
try {
XContentBuilder builder = createBuilder(pretty, human);
builder.startObject();
builder.field("error", "error building toString out of XContent: " + e.getMessage());
builder.field("stack_trace", ExceptionsHelper.stackTrace(e));
builder.endObject();
return builder.string();
} catch (IOException e2) {
throw new ElasticsearchException("cannot generate error message for deserialization", e);
}
}
}
private static XContentBuilder createBuilder(boolean pretty, boolean human) throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder();
if (pretty) {
builder.prettyPrint();
}
if (human) {
builder.humanReadable(true);
}
return builder;
}
/**
* Truncates string to a length less than length. Backtracks to throw out
* high surrogates.

47
core/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java
View File

@ -19,7 +19,6 @@
package org.elasticsearch.common.xcontent;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.Tuple;
@ -36,8 +35,6 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS;
@SuppressWarnings("unchecked")
public class XContentHelper {
@ -175,50 +172,6 @@ public class XContentHelper {
}
}
/**
* Writes serialized toXContent to pretty-printed JSON string.
*
* @param toXContent object to be pretty printed
* @return pretty-printed JSON serialization
*/
public static String toString(ToXContent toXContent) {
return toString(toXContent, EMPTY_PARAMS);
}
/**
* Writes serialized toXContent to pretty-printed JSON string.
*
* @param toXContent object to be pretty printed
* @param params serialization parameters
* @return pretty-printed JSON serialization
*/
public static String toString(ToXContent toXContent, Params params) {
try {
XContentBuilder builder = XContentFactory.jsonBuilder();
if (params.paramAsBoolean("pretty", true)) {
builder.prettyPrint();
}
if (params.paramAsBoolean("human", true)) {
builder.humanReadable(true);
}
builder.startObject();
toXContent.toXContent(builder, params);
builder.endObject();
return builder.string();
} catch (IOException e) {
try {
XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
builder.startObject();
builder.field("error", e.getMessage());
builder.endObject();
return builder.string();
} catch (IOException e2) {
throw new ElasticsearchException("cannot generate error message for deserialization", e);
}
}
}
/**
* Updates the provided changes into the source. If the key exists in the changes, it overrides the one in source
* unless both are Maps, in which case it recursively updated it.

3
core/src/test/java/org/elasticsearch/common/StringsTests.java
View File

@ -81,7 +81,8 @@ public class StringsTests extends ESTestCase {
String toString = Strings.toString(toXContent);
if (error) {
assertThat(toString, containsString("Error building toString out of XContent"));
assertThat(toString, containsString("\"error\":\"error building toString out of XContent:"));
assertThat(toString, containsString("\"stack_trace\":"));
} else {
assertThat(toString, containsString("\"ok\":\"here\""));
assertThat(toString, containsString("\"catastrophe\":\"\""));

10
core/src/test/java/org/elasticsearch/search/SearchCancellationIT.java
View File

@ -28,11 +28,10 @@ import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchScrollAction;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.script.MockScriptPlugin;
@ -61,9 +60,6 @@ import static org.hamcrest.Matchers.hasSize;
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE)
public class SearchCancellationIT extends ESIntegTestCase {
private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false"));
@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singleton(ScriptedBlockPlugin.class);
@ -154,7 +150,7 @@ public class SearchCancellationIT extends ESIntegTestCase {
awaitForBlock(plugins);
cancelSearch(SearchAction.NAME);
disableBlocks(plugins);
logger.info("Segments {}", XContentHelper.toString(client().admin().indices().prepareSegments("test").get(), FORMAT_PARAMS));
logger.info("Segments {}", Strings.toString(client().admin().indices().prepareSegments("test").get()));
ensureSearchWasCancelled(searchResponse);
}
@ -172,7 +168,7 @@ public class SearchCancellationIT extends ESIntegTestCase {
awaitForBlock(plugins);
cancelSearch(SearchAction.NAME);
disableBlocks(plugins);
logger.info("Segments {}", XContentHelper.toString(client().admin().indices().prepareSegments("test").get(), FORMAT_PARAMS));
logger.info("Segments {}", Strings.toString(client().admin().indices().prepareSegments("test").get()));
ensureSearchWasCancelled(searchResponse);
}

3
test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
View File

@ -97,7 +97,6 @@ import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
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.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
@ -1071,7 +1070,7 @@ public abstract class ESIntegTestCase extends ESTestCase {
* Prints current memory stats as info logging.
*/
public void logMemoryStats() {
logger.info("memory: {}", XContentHelper.toString(client().admin().cluster().prepareNodesStats().clear().setJvm(true).get()));
logger.info("memory: {}", Strings.toString(client().admin().cluster().prepareNodesStats().clear().setJvm(true).get(), true, true));
}
protected void ensureClusterSizeConsistency() {

45
test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
View File

@ -19,6 +19,25 @@
package org.elasticsearch.test.rest.yaml;
import com.carrotsearch.randomizedtesting.RandomizedTest;
import org.elasticsearch.client.http.HttpHost;
import org.elasticsearch.Version;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection;
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSuite;
import org.elasticsearch.test.rest.yaml.section.DoSection;
import org.elasticsearch.test.rest.yaml.section.ExecutableSection;
import org.junit.AfterClass;
import org.junit.Before;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@ -30,29 +49,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import com.carrotsearch.randomizedtesting.RandomizedTest;
import org.elasticsearch.client.http.HttpHost;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.Version;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection;
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSuite;
import org.elasticsearch.test.rest.yaml.section.DoSection;
import org.elasticsearch.test.rest.yaml.section.ExecutableSection;
import org.junit.AfterClass;
import org.junit.Before;
/**
* Runs a suite of yaml tests shared with all the official Elasticsearch clients against against an elasticsearch cluster.
*/
@ -147,7 +143,8 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
protected void afterIfFailed(List<Throwable> errors) {
// Dump the stash on failure. Instead of dumping it in true json we escape `\n`s so stack traces are easier to read
logger.info("Stash dump on failure [{}]",
XContentHelper.toString(restTestExecutionContext.stash()).replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
Strings.toString(restTestExecutionContext.stash(), true, true)
.replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
super.afterIfFailed(errors);
}