Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion buildSrc/src/main/resources/checkstyle_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]fielddata[/\\]plain[/\\]ParentChildIndexFieldData.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]fielddata[/\\]plain[/\\]SortedNumericDVIndexFieldData.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]fielddata[/\\]plain[/\\]SortedSetDVOrdinalsIndexFieldData.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]get[/\\]GetResult.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]get[/\\]ShardGetService.java" checks="LineLength" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]DocumentFieldMappers.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]DocumentMapper.java" checks="LineLength" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

import static java.util.Collections.unmodifiableMap;
import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_UUID_NA_VALUE;
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField;

/**
Expand Down Expand Up @@ -357,7 +357,8 @@ public static void toXContent(XContentBuilder builder, Params params, Throwable
* instances.
*/
public static ElasticsearchException fromXContent(XContentParser parser) throws IOException {
XContentParser.Token token = ensureFieldName(parser.nextToken(), parser::getTokenLocation);
XContentParser.Token token = parser.nextToken();
ensureExpectedToken(XContentParser.Token.FIELD_NAME, token, parser::getTokenLocation);

String type = null, reason = null, stack = null;
ElasticsearchException cause = null;
Expand Down
26 changes: 25 additions & 1 deletion core/src/main/java/org/elasticsearch/action/get/GetResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.get.GetField;
import org.elasticsearch.index.get.GetResult;

import java.io.IOException;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;

/**
* The response of a get action.
Expand All @@ -42,7 +44,7 @@
*/
public class GetResponse extends ActionResponse implements Iterable<GetField>, ToXContent {

private GetResult getResult;
GetResult getResult;

GetResponse() {
}
Expand Down Expand Up @@ -156,6 +158,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return getResult.toXContent(builder, params);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand this: GetResult#toXContent outputs another start/endObject token pair now. Why doesn't this affect the current behaviour of GetResponse?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the only user of GetResponse#toxContent was RestGetAction which would output start and end object by itself, I moved that out of the rest action to the GetResult object.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also MultiGetResponse was using this, but I just removed the startObject and endObject from it.

}

public static GetResponse fromXContent(XContentParser parser) throws IOException {
GetResult getResult = GetResult.fromXContent(parser);
return new GetResponse(getResult);
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
Expand All @@ -168,6 +175,23 @@ public void writeTo(StreamOutput out) throws IOException {
getResult.writeTo(out);
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
GetResponse getResponse = (GetResponse) o;
return Objects.equals(getResult, getResponse.getResult);
}

@Override
public int hashCode() {
return Objects.hash(getResult);
}

@Override
public String toString() {
return Strings.toString(this, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.endObject();
} else {
GetResponse getResponse = response.getResponse();
builder.startObject();
getResponse.toXContent(builder, params);
builder.endObject();
}
}
builder.endArray();
Expand All @@ -154,9 +152,6 @@ static final class Fields {
static final String _INDEX = "_index";
static final String _TYPE = "_type";
static final String _ID = "_id";
static final String ERROR = "error";
static final String ROOT_CAUSE = "root_cause";

}

@Override
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/elasticsearch/common/text/Text.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ public int hashCode() {

@Override
public boolean equals(Object obj) {
if (obj == null) {
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
return bytes().equals(((Text) obj).bytes());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,22 @@ public static void writeRawField(String field, BytesReference source, XContentBu
builder.rawField(field, source);
}
}

/**
* Returns the bytes that represent the XContent output of the provided {@link ToXContent} object, using the provided
* {@link XContentType}. Wraps the output into a new anonymous object depending on the value of the wrapInObject argument.
*/
public static BytesReference toXContent(ToXContent toXContent, XContentType xContentType, boolean wrapInObject) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done after merging: 87d8764

try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
if (wrapInObject) {
builder.startObject();
}
toXContent.toXContent(builder, ToXContent.EMPTY_PARAMS);
if (wrapInObject) {
builder.endObject();
}
return builder.bytes();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContentParser.Token;

import java.io.IOException;
import java.util.Locale;
import java.util.function.Supplier;

Expand All @@ -35,34 +34,6 @@ public final class XContentParserUtils {
private XContentParserUtils() {
}

/**
* Makes sure that current token is of type {@link XContentParser.Token#FIELD_NAME}
*
* @return the token
* @throws ParsingException if the token is not of type {@link XContentParser.Token#FIELD_NAME}
*/
public static Token ensureFieldName(Token token, Supplier<XContentLocation> location) throws IOException {
return ensureType(Token.FIELD_NAME, token, location);
}

/**
* Makes sure that current token is of type {@link XContentParser.Token#FIELD_NAME} and the the field name is equal to the provided one
*
* @return the token
* @throws ParsingException if the token is not of type {@link XContentParser.Token#FIELD_NAME} or is not equal to the given
* field name
*/
public static Token ensureFieldName(XContentParser parser, Token token, String fieldName) throws IOException {
Token t = ensureType(Token.FIELD_NAME, token, parser::getTokenLocation);

String current = parser.currentName() != null ? parser.currentName() : "<null>";
if (current.equals(fieldName) == false) {
String message = "Failed to parse object: expecting field with name [%s] but found [%s]";
throw new ParsingException(parser.getTokenLocation(), String.format(Locale.ROOT, message, fieldName, current));
}
return t;
}

/**
* @throws ParsingException with a "unknown field found" reason
*/
Expand All @@ -72,16 +43,14 @@ public static void throwUnknownField(String field, XContentLocation location) {
}

/**
* Makes sure that current token is of the expected type
* Makes sure that provided token is of the expected type
*
* @return the token
* @throws ParsingException if the token is not equal to the expected type
*/
private static Token ensureType(Token expected, Token current, Supplier<XContentLocation> location) {
if (current != expected) {
public static void ensureExpectedToken(Token expected, Token actual, Supplier<XContentLocation> location) {
if (actual != expected) {
String message = "Failed to parse object: expecting token of type [%s] but found [%s]";
throw new ParsingException(location.get(), String.format(Locale.ROOT, message, expected, current));
throw new ParsingException(location.get(), String.format(Locale.ROOT, message, expected, actual));
}
return current;
}
}
77 changes: 74 additions & 3 deletions core/src/main/java/org/elasticsearch/index/get/GetField.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,25 @@

package org.elasticsearch.index.get;

import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Streamable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;

public class GetField implements Streamable, Iterable<Object> {
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;

public class GetField implements Streamable, ToXContent, Iterable<Object> {

private String name;
private List<Object> values;
Expand All @@ -38,8 +46,8 @@ private GetField() {
}

public GetField(String name, List<Object> values) {
this.name = name;
this.values = values;
this.name = Objects.requireNonNull(name, "name must not be null");
this.values = Objects.requireNonNull(values, "values must not be null");
}

public String getName() {
Expand Down Expand Up @@ -90,4 +98,67 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeGenericValue(obj);
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startArray(name);
for (Object value : values) {
//this call doesn't really need to support writing any kind of object.
//Stored fields values are converted using MappedFieldType#valueForDisplay.
//As a result they can either be Strings, Numbers, Booleans, or BytesReference, that's all.
builder.value(value);
}
builder.endArray();
return builder;
}

public static GetField fromXContent(XContentParser parser) throws IOException {
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation);
String fieldName = parser.currentName();
XContentParser.Token token = parser.nextToken();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this reference is not really useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

ensureExpectedToken(XContentParser.Token.START_ARRAY, token, parser::getTokenLocation);
List<Object> values = new ArrayList<>();
while((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
Object value;
if (token == XContentParser.Token.VALUE_STRING) {
value = parser.text();
} else if (token == XContentParser.Token.VALUE_NUMBER) {
value = parser.numberValue();
} else if (token == XContentParser.Token.VALUE_BOOLEAN) {
value = parser.booleanValue();
} else if (token == XContentParser.Token.VALUE_EMBEDDED_OBJECT) {
value = new BytesArray(parser.binaryValue());
} else {
throw new ParsingException(parser.getTokenLocation(), "Failed to parse object: unsupported token found [" + token + "]");
}
values.add(value);
}
return new GetField(fieldName, values);
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
GetField objects = (GetField) o;
return Objects.equals(name, objects.name) &&
Objects.equals(values, objects.values);
}

@Override
public int hashCode() {
return Objects.hash(name, values);
}

@Override
public String toString() {
return "GetField{" +
"name='" + name + '\'' +
", values=" + values +
'}';
}
}
Loading