Skip to content

Commit

Permalink
Clarify some ToXContent implementations behaviour (#41000)
Browse files Browse the repository at this point in the history
This change adds either ToXContentObject or ToXContentFragment to classes
directly implementing ToXContent currently. This helps in reasoning about
whether those implementations output full xcontent object or just fragments.

Relates to #16347
  • Loading branch information
Christoph Büscher committed Apr 15, 2019
1 parent 5ef247d commit 2980a6c
Show file tree
Hide file tree
Showing 31 changed files with 120 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startArray(FAILURES.getPreferredName());
if (shardFailures != null) {
for (ShardOperationFailedException shardFailure : shardFailures) {
builder.startObject();
shardFailure.toXContent(builder, params);
builder.endObject();
}
}
builder.endArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
Expand All @@ -56,7 +56,7 @@
/**
* A request to create an index template.
*/
public class PutIndexTemplateRequest extends MasterNodeRequest<PutIndexTemplateRequest> implements IndicesRequest, ToXContent {
public class PutIndexTemplateRequest extends MasterNodeRequest<PutIndexTemplateRequest> implements IndicesRequest, ToXContentFragment {

private String name;

Expand Down Expand Up @@ -191,7 +191,7 @@ public PutIndexTemplateRequest settings(Map<String, Object> source) {
public Settings settings() {
return this.settings;
}

/**
* Adds mapping that will be added when the index gets created.
*
Expand All @@ -201,7 +201,7 @@ public Settings settings() {
public PutIndexTemplateRequest mapping(String source, XContentType xContentType) {
internalMapping(XContentHelper.convertToMap(new BytesArray(source), true, xContentType).v2());
return this;
}
}

/**
* The cause for this index template creation.
Expand All @@ -221,11 +221,11 @@ public String cause() {
* @param source The mapping source
*/
public PutIndexTemplateRequest mapping(XContentBuilder source) {
internalMapping(XContentHelper.convertToMap(BytesReference.bytes(source),
internalMapping(XContentHelper.convertToMap(BytesReference.bytes(source),
true, source.contentType()).v2());
return this;
}
return this;
}

/**
* Adds mapping that will be added when the index gets created.
*
Expand All @@ -235,16 +235,16 @@ public PutIndexTemplateRequest mapping(XContentBuilder source) {
public PutIndexTemplateRequest mapping(BytesReference source, XContentType xContentType) {
internalMapping(XContentHelper.convertToMap(source, true, xContentType).v2());
return this;
}
}

/**
* Adds mapping that will be added when the index gets created.
*
* @param source The mapping source
*/
public PutIndexTemplateRequest mapping(Map<String, Object> source) {
return internalMapping(source);
}
}

private PutIndexTemplateRequest internalMapping(Map<String, Object> source) {
try {
Expand All @@ -257,12 +257,12 @@ private PutIndexTemplateRequest internalMapping(Map<String, Object> source) {
return this;
} catch (IOException e) {
throw new UncheckedIOException("failed to convert source to json", e);
}
}
} catch (IOException e) {
throw new ElasticsearchGenerationException("Failed to generate [" + source + "]", e);
}
}
}

public BytesReference mappings() {
return this.mappings;
}
Expand Down Expand Up @@ -349,8 +349,8 @@ public PutIndexTemplateRequest source(byte[] source, int offset, int length, XCo
*/
public PutIndexTemplateRequest source(BytesReference source, XContentType xContentType) {
return source(XContentHelper.convertToMap(source, true, xContentType).v2());
}
}


public Set<Alias> aliases() {
return this.aliases;
Expand Down Expand Up @@ -441,7 +441,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.copyCurrentStructure(parser);
}
}

builder.startObject("aliases");
for (Alias alias : aliases) {
alias.toXContent(builder, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
Expand All @@ -34,7 +34,7 @@
import java.util.Objects;
import java.util.Optional;

public class FindFileStructureRequest implements Validatable, ToXContent {
public class FindFileStructureRequest implements Validatable, ToXContentFragment {

public static final ParseField LINES_TO_SAMPLE = new ParseField("lines_to_sample");
public static final ParseField TIMEOUT = new ParseField("timeout");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.client;

import com.fasterxml.jackson.core.JsonParseException;

import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
Expand Down Expand Up @@ -61,6 +62,7 @@
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.cbor.CborXContent;
Expand Down Expand Up @@ -176,7 +178,7 @@ public void testInfo() throws IOException {
MainResponse testInfo = new MainResponse("nodeName", new MainResponse.Version("number", "buildFlavor", "buildType", "buildHash",
"buildDate", true, "luceneVersion", "minimumWireCompatibilityVersion", "minimumIndexCompatibilityVersion"),
"clusterName", "clusterUuid", "You Know, for Search");
mockResponse((builder, params) -> {
mockResponse((ToXContentFragment) (builder, params) -> {
// taken from the server side MainResponse
builder.field("name", testInfo.getNodeName());
builder.field("cluster_name", testInfo.getClusterName());
Expand Down Expand Up @@ -762,12 +764,12 @@ public void testApiNamingConventions() throws Exception {
Collectors.mapping(Tuple::v2, Collectors.toSet())));

// TODO remove in 8.0 - we will undeprecate indices.get_template because the current getIndexTemplate
// impl will replace the existing getTemplate method.
// impl will replace the existing getTemplate method.
// The above general-purpose code ignores all deprecated methods which in this case leaves `getTemplate`
// looking like it doesn't have a valid implementatation when it does.
// looking like it doesn't have a valid implementatation when it does.
apiUnsupported.remove("indices.get_template");



for (Map.Entry<String, Set<Method>> entry : methods.entrySet()) {
String apiName = entry.getKey();
Expand Down Expand Up @@ -830,7 +832,7 @@ private static void assertSyncMethod(Method method, String apiName, List<String>
assertThat("the return type for method [" + method + "] is incorrect",
method.getReturnType().getSimpleName(), equalTo("boolean"));
} else {
// It's acceptable for 404s to be represented as empty Optionals
// It's acceptable for 404s to be represented as empty Optionals
if (!method.getReturnType().isAssignableFrom(Optional.class)) {
assertThat("the return type for method [" + method + "] is incorrect",
method.getReturnType().getSimpleName(), endsWith("Response"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
Expand Down Expand Up @@ -107,7 +106,7 @@ public Response newResponse() {
return new Response();
}

public static class Request extends SingleShardRequest<Request> implements ToXContent {
public static class Request extends SingleShardRequest<Request> implements ToXContentObject {

private static final ParseField SCRIPT_FIELD = new ParseField("script");
private static final ParseField CONTEXT_FIELD = new ParseField("context");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.io.stream.Streamable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.rest.RestStatus;

import java.util.Objects;
Expand All @@ -30,7 +30,7 @@
* An exception indicating that a failure occurred performing an operation on the shard.
*
*/
public abstract class ShardOperationFailedException implements Streamable, ToXContent {
public abstract class ShardOperationFailedException implements Streamable, ToXContentObject {

protected String index;
protected int shardId = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
Expand All @@ -36,7 +36,7 @@

import static org.elasticsearch.action.ValidateActions.addValidationError;

public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptRequest> implements ToXContent {
public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptRequest> implements ToXContentFragment {

private String id;
private String context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.carrotsearch.hppc.cursors.IntObjectCursor;
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
Expand Down Expand Up @@ -267,8 +268,10 @@ public void writeTo(StreamOutput out) throws IOException {

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field("node", nodeId());
super.toXContent(builder, params);
super.innerToXContent(builder, params);
builder.endObject();
return builder;
}
}
Expand Down Expand Up @@ -361,9 +364,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (failures.size() > 0) {
builder.startArray(Fields.FAILURES);
for (Failure failure : failures) {
builder.startObject();
failure.toXContent(builder, params);
builder.endObject();
}
builder.endArray();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
Expand All @@ -60,14 +60,14 @@
import java.util.stream.Collectors;

import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;

/**
* A request to create an index template.
*/
public class PutIndexTemplateRequest extends MasterNodeRequest<PutIndexTemplateRequest> implements IndicesRequest, ToXContent {
public class PutIndexTemplateRequest extends MasterNodeRequest<PutIndexTemplateRequest> implements IndicesRequest, ToXContentObject {

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(PutIndexTemplateRequest.class));

Expand Down Expand Up @@ -519,32 +519,35 @@ public void writeTo(StreamOutput out) throws IOException {

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.field("index_patterns", indexPatterns);
builder.field("order", order);
if (version != null) {
builder.field("version", version);
}
builder.startObject();
{
builder.field("index_patterns", indexPatterns);
builder.field("order", order);
if (version != null) {
builder.field("version", version);
}

builder.startObject("settings");
settings.toXContent(builder, params);
builder.endObject();
builder.startObject("settings");
settings.toXContent(builder, params);
builder.endObject();

builder.startObject("mappings");
for (Map.Entry<String, String> entry : mappings.entrySet()) {
builder.field(entry.getKey());
try (XContentParser parser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entry.getValue())) {
builder.copyCurrentStructure(parser);
builder.startObject("mappings");
for (Map.Entry<String, String> entry : mappings.entrySet()) {
builder.field(entry.getKey());
try (XContentParser parser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entry.getValue())) {
builder.copyCurrentStructure(parser);
}
}
}
builder.endObject();
builder.endObject();

builder.startObject("aliases");
for (Alias alias : aliases) {
alias.toXContent(builder, params);
builder.startObject("aliases");
for (Alias alias : aliases) {
alias.toXContent(builder, params);
}
builder.endObject();
}
builder.endObject();

return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ protected void metadataToXContent(XContentBuilder builder, Params params) throws
builder.startArray();
ShardOperationFailedException[] failures = ExceptionsHelper.groupBy(shardFailures);
for (ShardOperationFailedException failure : failures) {
builder.startObject();
failure.toXContent(builder, params);
builder.endObject();
}
builder.endArray();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
Expand Down Expand Up @@ -408,7 +408,7 @@ public String toString() {
* Holds info about the clusters that the search was executed on: how many in total, how many of them were successful
* and how many of them were skipped.
*/
public static class Clusters implements ToXContent, Writeable {
public static class Clusters implements ToXContentFragment, Writeable {

public static final Clusters EMPTY = new Clusters(0, 0, 0);

Expand Down
Loading

0 comments on commit 2980a6c

Please sign in to comment.