Skip to content

Fixed naming inconsistency for fields/stored_fields in the APIs #20166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 13, 2016
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void handleRequest(final RestRequest request, final RestChannel channel,
}
bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT));
bulkRequest.setRefreshPolicy(request.param("refresh"));
bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, defaultPipeline, null, true);
bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, null, defaultPipeline, null, true);

// short circuit the call to the transport layer
BulkRestBuilderListener listener = new BulkRestBuilderListener(channel, request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nu
}

public synchronized BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultPipeline, @Nullable Object payload) throws Exception {
bulkRequest.add(data, defaultIndex, defaultType, null, null, defaultPipeline, payload, true);
bulkRequest.add(data, defaultIndex, defaultType, null, null, null, defaultPipeline, payload, true);
executeIfNeeded();
return this;
}
Expand Down
23 changes: 19 additions & 4 deletions core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,21 @@
import org.elasticsearch.action.support.replication.ReplicationRequest;
import org.elasticsearch.action.update.UpdateRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -57,6 +61,8 @@
* @see org.elasticsearch.client.Client#bulk(BulkRequest)
*/
public class BulkRequest extends ActionRequest<BulkRequest> implements CompositeIndicesRequest, WriteRequest<BulkRequest> {
private static final DeprecationLogger DEPRECATION_LOGGER =
new DeprecationLogger(Loggers.getLogger(BulkRequest.class));

private static final int REQUEST_OVERHEAD = 50;

Expand Down Expand Up @@ -257,17 +263,17 @@ public BulkRequest add(byte[] data, int from, int length, @Nullable String defau
* Adds a framed data in binary format
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType) throws Exception {
return add(data, defaultIndex, defaultType, null, null, null, null, true);
return add(data, defaultIndex, defaultType, null, null, null, null, null, true);
}

/**
* Adds a framed data in binary format
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex) throws Exception {
return add(data, defaultIndex, defaultType, null, null, null, null, allowExplicitIndex);
return add(data, defaultIndex, defaultType, null, null, null, null, null, allowExplicitIndex);
}

public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws Exception {
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws Exception {
XContent xContent = XContentFactory.xContent(data);
int line = 0;
int from = 0;
Expand Down Expand Up @@ -301,6 +307,7 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
String id = null;
String routing = defaultRouting;
String parent = null;
FetchSourceContext fetchSourceContext = defaultFetchSourceContext;
String[] fields = defaultFields;
String timestamp = null;
TimeValue ttl = null;
Expand Down Expand Up @@ -353,16 +360,21 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
pipeline = parser.text();
} else if ("fields".equals(currentFieldName)) {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains a simple value for parameter [fields] while a list is expected");
} else if ("_source".equals(currentFieldName)) {
fetchSourceContext = FetchSourceContext.parse(parser, ParseFieldMatcher.EMPTY);
} else {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains an unknown parameter [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("fields".equals(currentFieldName)) {
DEPRECATION_LOGGER.deprecated("Deprecated field [fields] used, expected [_source] instead");
List<Object> values = parser.list();
fields = values.toArray(new String[values.size()]);
} else {
throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
}
} else if (token == XContentParser.Token.START_OBJECT && "_source".equals(currentFieldName)) {
fetchSourceContext = FetchSourceContext.parse(parser, ParseFieldMatcher.STRICT);
} else if (token != XContentParser.Token.VALUE_NULL) {
throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
}
Expand Down Expand Up @@ -402,7 +414,10 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
.version(version).versionType(versionType)
.routing(routing)
.parent(parent)
.source(data.slice(from, nextMarker - from));
.fromXContent(data.slice(from, nextMarker - from));
if (fetchSourceContext != null) {
updateRequest.fetchSource(fetchSourceContext);
}
if (fields != null) {
updateRequest.fields(fields);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ private Tuple<Translog.Location, BulkItemRequest> update(IndexMetaData metaData,
// add the response
IndexResponse indexResponse = result.getResponse();
UpdateResponse updateResponse = new UpdateResponse(indexResponse.getShardInfo(), indexResponse.getShardId(), indexResponse.getType(), indexResponse.getId(), indexResponse.getVersion(), indexResponse.getResult());
if (updateRequest.fields() != null && updateRequest.fields().length > 0) {
if ((updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) ||
(updateRequest.fields() != null && updateRequest.fields().length > 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you indent this line once more so it doesn't look like part of the body of the if statement?

Tuple<XContentType, Map<String, Object>> sourceAndContent = XContentHelper.convertToMap(indexSourceAsBytes, true);
updateResponse.setGetResult(updateHelper.extractGetResult(updateRequest, request.index(), indexResponse.getVersion(), sourceAndContent.v2(), sourceAndContent.v1(), indexSourceAsBytes));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
private String routing;
private String preference;
private QueryBuilder query;
private String[] fields;
private String[] storedFields;
private FetchSourceContext fetchSourceContext;

private String[] filteringAlias = Strings.EMPTY_ARRAY;
Expand Down Expand Up @@ -122,12 +122,12 @@ public FetchSourceContext fetchSourceContext() {
}


public String[] fields() {
return fields;
public String[] storedFields() {
return storedFields;
}

public ExplainRequest fields(String[] fields) {
this.fields = fields;
public ExplainRequest storedFields(String[] fields) {
this.storedFields = fields;
return this;
}

Expand Down Expand Up @@ -167,8 +167,8 @@ public void readFrom(StreamInput in) throws IOException {
preference = in.readOptionalString();
query = in.readNamedWriteable(QueryBuilder.class);
filteringAlias = in.readStringArray();
fields = in.readOptionalStringArray();
fetchSourceContext = in.readOptionalStreamable(FetchSourceContext::new);
storedFields = in.readOptionalStringArray();
fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::new);
nowInMillis = in.readVLong();
}

Expand All @@ -181,8 +181,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(preference);
out.writeNamedWriteable(query);
out.writeStringArray(filteringAlias);
out.writeOptionalStringArray(fields);
out.writeOptionalStreamable(fetchSourceContext);
out.writeOptionalStringArray(storedFields);
out.writeOptionalWriteable(fetchSourceContext);
out.writeVLong(nowInMillis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ public ExplainRequestBuilder setQuery(QueryBuilder query) {
}

/**
* Explicitly specify the fields that will be returned for the explained document. By default, nothing is returned.
* Explicitly specify the stored fields that will be returned for the explained document. By default, nothing is returned.
*/
public ExplainRequestBuilder setFields(String... fields) {
request.fields(fields);
public ExplainRequestBuilder setStoredFields(String... fields) {
request.storedFields(fields);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,11 @@ protected ExplainResponse shardOperation(ExplainRequest request, ShardId shardId
Rescorer rescorer = ctx.rescorer();
explanation = rescorer.explain(topLevelDocId, context, ctx, explanation);
}
if (request.fields() != null || (request.fetchSourceContext() != null && request.fetchSourceContext().fetchSource())) {
if (request.storedFields() != null || (request.fetchSourceContext() != null && request.fetchSourceContext().fetchSource())) {
// Advantage is that we're not opening a second searcher to retrieve the _source. Also
// because we are working in the same searcher in engineGetResult we can be sure that a
// doc isn't deleted between the initial get and this call.
GetResult getResult = context.indexShard().getService().get(result, request.id(), request.type(), request.fields(),
request.fetchSourceContext());
GetResult getResult = context.indexShard().getService().get(result, request.id(), request.type(), request.storedFields(), request.fetchSourceContext());
return new ExplainResponse(shardId.getIndexName(), request.type(), request.id(), true, explanation, getResult);
} else {
return new ExplainResponse(shardId.getIndexName(), request.type(), request.id(), true, explanation);
Expand Down
35 changes: 11 additions & 24 deletions core/src/main/java/org/elasticsearch/action/get/GetRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
private String parent;
private String preference;

private String[] fields;
private String[] storedFields;

private FetchSourceContext fetchSourceContext;

Expand Down Expand Up @@ -186,20 +186,20 @@ public FetchSourceContext fetchSourceContext() {
}

/**
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* Explicitly specify the stored fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public GetRequest fields(String... fields) {
this.fields = fields;
public GetRequest storedFields(String... fields) {
this.storedFields = fields;
return this;
}

/**
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* Explicitly specify the stored fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public String[] fields() {
return this.fields;
public String[] storedFields() {
return this.storedFields;
}

/**
Expand Down Expand Up @@ -260,18 +260,12 @@ public void readFrom(StreamInput in) throws IOException {
parent = in.readOptionalString();
preference = in.readOptionalString();
refresh = in.readBoolean();
int size = in.readInt();
if (size >= 0) {
fields = new String[size];
for (int i = 0; i < size; i++) {
fields[i] = in.readString();
}
}
storedFields = in.readOptionalStringArray();
realtime = in.readBoolean();

this.versionType = VersionType.fromValue(in.readByte());
this.version = in.readLong();
fetchSourceContext = in.readOptionalStreamable(FetchSourceContext::new);
fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::new);
}

@Override
Expand All @@ -284,18 +278,11 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(preference);

out.writeBoolean(refresh);
if (fields == null) {
out.writeInt(-1);
} else {
out.writeInt(fields.length);
for (String field : fields) {
out.writeString(field);
}
}
out.writeOptionalStringArray(storedFields);
out.writeBoolean(realtime);
out.writeByte(versionType.getValue());
out.writeLong(version);
out.writeOptionalStreamable(fetchSourceContext);
out.writeOptionalWriteable(fetchSourceContext);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public GetRequestBuilder setPreference(String preference) {
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public GetRequestBuilder setFields(String... fields) {
request.fields(fields);
public GetRequestBuilder setStoredFields(String... fields) {
request.storedFields(fields);
return this;
}

Expand Down
12 changes: 12 additions & 0 deletions core/src/main/java/org/elasticsearch/action/get/GetResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,26 @@ public Map<String, Object> getSource() {
return getResult.getSource();
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
public Map<String, GetField> getFields() {
return getResult.getFields();
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
public GetField getField(String name) {
return getResult.field(name);
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
@Override
public Iterator<GetField> iterator() {
return getResult.iterator();
Expand Down
Loading