Skip to content

Using ObjectParser in UpdateRequest #29293

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.

Sign up for GitHub

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 8 commits into from
Apr 16, 2018
Merged

Using ObjectParser in UpdateRequest #29293

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ce966d0
Using ObjectParser in UpdateRequest (#28740)
liketic Mar 29, 2018
0d3fd89
Merge branch 'master' into fix-issues/28740
liketic Apr 2, 2018
a3a8087
Address comments
liketic Apr 2, 2018
d456759
Drop parameter fields
liketic Apr 2, 2018
2024751
Fix test bwc issue
liketic Apr 3, 2018
fae7908
Fix bwc issue caused by removing fields
liketic Apr 4, 2018
553bf99
Merge branch 'master' into fix-issues/28740
liketic Apr 4, 2018
28622b4
Resolve conflicts
liketic Apr 12, 2018
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
100 changes: 56 additions & 44 deletions server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.action.update;

import java.util.Arrays;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.index.IndexRequest;
Expand All @@ -28,11 +27,13 @@
import org.elasticsearch.action.support.replication.ReplicationRequest;
import org.elasticsearch.action.support.single.instance.InstanceShardOperationRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
Expand All @@ -46,6 +47,7 @@
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -55,6 +57,58 @@

public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
implements DocWriteRequest<UpdateRequest>, WriteRequest<UpdateRequest>, ToXContentObject {
private static ObjectParser<UpdateRequest, Void> PARSER;

private static final ParseField SCRIPT_FIELD = new ParseField("script");
private static final ParseField SCRIPTED_UPSERT_FIELD = new ParseField("scripted_upsert");
private static final ParseField UPSERT_FIELD = new ParseField("upsert");
private static final ParseField DOC_FIELD = new ParseField("doc");
private static final ParseField DOC_AS_UPSERT_FIELD = new ParseField("doc_as_upsert");
private static final ParseField DETECT_NOOP_FIELD = new ParseField("detect_noop");
private static final ParseField FIELDS_FIELD = new ParseField("fields");
private static final ParseField SOURCE_FIELD = new ParseField("_source");

static {
PARSER = new ObjectParser<>(UpdateRequest.class.getSimpleName());
PARSER.declareField((request, script) -> request.script = script,
(parser, context) -> Script.parse(parser), SCRIPT_FIELD, ObjectParser.ValueType.OBJECT_OR_STRING);
PARSER.declareBoolean(UpdateRequest::scriptedUpsert, SCRIPTED_UPSERT_FIELD);

PARSER.declareObject((request, builder) -> request.safeUpsertRequest().source(builder),
(parser, context) -> {
XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType());
builder.copyCurrentStructure(parser);
return builder;
}, UPSERT_FIELD);

PARSER.declareObject((request, builder) -> request.safeDoc().source(builder),
(parser, context) -> {
XContentBuilder docBuilder = XContentFactory.contentBuilder(parser.contentType());
docBuilder.copyCurrentStructure(parser);
return docBuilder;
}, DOC_FIELD);

PARSER.declareBoolean(UpdateRequest::docAsUpsert, DOC_AS_UPSERT_FIELD);
PARSER.declareBoolean(UpdateRequest::detectNoop, DETECT_NOOP_FIELD);
PARSER.declareField((request, fields) -> {
if (fields != null) {
request.fields(fields.toArray(new String[fields.size()]));
}
},
(parser, context) -> {
XContentParser.Token token = parser.currentToken();
List<Object> fields = null;
if (token == XContentParser.Token.START_ARRAY) {
fields = parser.list();
} else if (token.isValue()) {
fields = Collections.singletonList(parser.text());
}
return fields;
}, FIELDS_FIELD, ObjectParser.ValueType.STRING_ARRAY);
Copy link
Member

@nik9000 nik9000 Mar 30, 2018

Choose a reason for hiding this comment

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

I think you can use PARSER.declareStringArray(UpdateRequest::fields, FIELDS_FIELD). It doens't reject non-array things but I think that is fine. Another choice would be to simply pitch this. It has been deprecated for a year and a half (since 1764ec5) and we are only going to merge this to the master branch anyway so it is going to be released during a major version so we can make breaking changes like dropping a deprecated field.

PARSER.declareField(UpdateRequest::fetchSource,
(parser, context) -> FetchSourceContext.fromXContent(parser), SOURCE_FIELD,
ObjectParser.ValueType.OBJECT_ARRAY_BOOLEAN_OR_STRING);
}

private String type;
private String id;
Expand Down Expand Up @@ -720,49 +774,7 @@ public boolean detectNoop() {
}

public UpdateRequest fromXContent(XContentParser parser) throws IOException {
Script script = null;
XContentParser.Token token = parser.nextToken();
if (token == null) {
return this;
}
String currentFieldName = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if ("script".equals(currentFieldName)) {
script = Script.parse(parser);
} else if ("scripted_upsert".equals(currentFieldName)) {
scriptedUpsert = parser.booleanValue();
} else if ("upsert".equals(currentFieldName)) {
XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType());
builder.copyCurrentStructure(parser);
safeUpsertRequest().source(builder);
} else if ("doc".equals(currentFieldName)) {
XContentBuilder docBuilder = XContentFactory.contentBuilder(parser.contentType());
docBuilder.copyCurrentStructure(parser);
safeDoc().source(docBuilder);
} else if ("doc_as_upsert".equals(currentFieldName)) {
docAsUpsert(parser.booleanValue());
} else if ("detect_noop".equals(currentFieldName)) {
detectNoop(parser.booleanValue());
} else if ("fields".equals(currentFieldName)) {
List<Object> fields = null;
if (token == XContentParser.Token.START_ARRAY) {
fields = (List) parser.list();
} else if (token.isValue()) {
fields = Collections.singletonList(parser.text());
}
if (fields != null) {
fields(fields.toArray(new String[fields.size()]));
}
} else if ("_source".equals(currentFieldName)) {
fetchSourceContext = FetchSourceContext.fromXContent(parser);
}
}
if (script != null) {
this.script = script;
}
return this;
return PARSER.parse(parser, this, null);
}

public boolean docAsUpsert() {
Expand Down
11 changes: 11 additions & 0 deletions server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,17 @@ public void testFieldsParsing() throws Exception {
assertThat(request.fields(), arrayContaining("field1", "field2"));
}

public void testUnknownFieldParsing() throws Exception {
UpdateRequest request = new UpdateRequest("test", "type", "1");
XContentParser contentParser = createParser(XContentFactory.jsonBuilder()
.startObject()
.field("unknown_field", "test")
.endObject());

IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> request.fromXContent(contentParser));
assertEquals("[UpdateRequest] unknown field [unknown_field], parser not found", ex.getMessage());
}

public void testFetchSourceParsing() throws Exception {
UpdateRequest request = new UpdateRequest("test", "type1", "1");
request.fromXContent(createParser(XContentFactory.jsonBuilder()
Expand Down