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

Added validation for upserd request (#24282) 

The version on an update request is a syntactic sugar
for get of a specific version, doc merge and a version
index. This changes it to reject requests with both
upsert and a version.
If the upsert index request is versioned, we also
reject the op.
This commit is contained in:
Kunal Kapoor 2017-04-28 17:32:09 +05:30 committed by Nik Everett
parent a72db191f2
commit a5bd2012b6
4 changed files with 60 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
View File

@ -98,6 +98,12 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = super.validate();
if (version != Versions.MATCH_ANY && upsertRequest != null) {
validationException = addValidationError("can't provide both upsert request and a version", validationException);
}
if(upsertRequest != null && upsertRequest.version() != Versions.MATCH_ANY) {
validationException = addValidationError("can't provide version in upsert request", validationException);
}
if (type == null) {
validationException = addValidationError("type is missing", validationException);
}

36
core/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java
View File

@ -40,6 +40,7 @@ import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -255,4 +256,39 @@ public class BulkRequestTests extends ESTestCase {
assertEquals(1, request.sourceAsMap().size());
assertEquals("value", request.sourceAsMap().get("field"));
}
public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOException {
XContentType xContentType = XContentType.SMILE;
BytesReference data;
try (BytesStreamOutput out = new BytesStreamOutput()) {
try (XContentBuilder builder = XContentFactory.contentBuilder(xContentType, out)) {
builder.startObject();
builder.startObject("update");
builder.field("_index", "index");
builder.field("_type", "type");
builder.field("_id", "id");
builder.field("_version", 1L);
builder.endObject();
builder.endObject();
}
out.write(xContentType.xContent().streamSeparator());
try(XContentBuilder builder = XContentFactory.contentBuilder(xContentType, out)) {
builder.startObject();
builder.field("doc", "{}");
Map<String,Object> values = new HashMap<>();
values.put("_version", 2L);
values.put("_index", "index");
values.put("_type", "type");
builder.field("upsert", values);
builder.endObject();
}
out.write(xContentType.xContent().streamSeparator());
data = out.bytes();
}
BulkRequest bulkRequest = new BulkRequest();
bulkRequest.add(data, null, null, xContentType);
assertThat(bulkRequest.validate().validationErrors(), contains("can't provide both upsert request and a version",
"can't provide version in upsert request"));
}
}

17
core/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java
View File

@ -59,9 +59,11 @@ import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
import static org.elasticsearch.common.xcontent.XContentHelper.update;
import static org.elasticsearch.script.MockScriptEngine.mockInlineScript;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;
@ -485,4 +487,19 @@ public class UpdateRequestTests extends ESTestCase {
BytesReference finalBytes = toXContent(parsedUpdateRequest, xContentType, humanReadable);
assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
}
public void testToValidateUpsertRequestAndVersion() {
UpdateRequest updateRequest = new UpdateRequest("index", "type", "id");
updateRequest.version(1L);
updateRequest.doc("{}", XContentType.JSON);
updateRequest.upsert(new IndexRequest("index","type", "id"));
assertThat(updateRequest.validate().validationErrors(), contains("can't provide both upsert request and a version"));
}
public void testToValidateUpsertRequestWithVersion() {
UpdateRequest updateRequest = new UpdateRequest("index", "type", "id");
updateRequest.doc("{}", XContentType.JSON);
updateRequest.upsert(new IndexRequest("index", "type", "1").version(1L));
assertThat(updateRequest.validate().validationErrors(), contains("can't provide version in upsert request"));
}
}

56
core/src/test/java/org/elasticsearch/update/UpdateIT.java
View File

@ -491,61 +491,7 @@ public class UpdateIT extends ESIntegTestCase {
assertThat(updateResponse.getGetResult().sourceAsMap().get("bar").toString(), equalTo("baz"));
assertThat(updateResponse.getGetResult().sourceAsMap().get("extra").toString(), equalTo("foo"));
}
public void testVersionedUpdate() throws Exception {
assertAcked(prepareCreate("test").addAlias(new Alias("alias")));
ensureGreen();
index("test", "type", "1", "text", "value"); // version is now 1
assertThrows(client().prepareUpdate(indexOrAlias(), "type", "1")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v2"))).setVersion(2)
.execute(),
VersionConflictEngineException.class);
client().prepareUpdate(indexOrAlias(), "type", "1")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v2"))).setVersion(1).get();
assertThat(client().prepareGet("test", "type", "1").get().getVersion(), equalTo(2L));
// and again with a higher version..
client().prepareUpdate(indexOrAlias(), "type", "1")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v3"))).setVersion(2).get();
assertThat(client().prepareGet("test", "type", "1").get().getVersion(), equalTo(3L));
// after delete
client().prepareDelete("test", "type", "1").get();
assertThrows(client().prepareUpdate("test", "type", "1")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v2"))).setVersion(3)
.execute(),
DocumentMissingException.class);
// external versioning
client().prepareIndex("test", "type", "2").setSource("text", "value").setVersion(10).setVersionType(VersionType.EXTERNAL).get();
assertThrows(client().prepareUpdate(indexOrAlias(), "type", "2")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v2"))).setVersion(2)
.setVersionType(VersionType.EXTERNAL).execute(),
ActionRequestValidationException.class);
GetResponse get = get("test", "type", "2");
assertThat(get.getVersion(), equalTo(10L));
assertThat((String) get.getSource().get("text"), equalTo("value"));
// upserts - the combination with versions is a bit weird. Test are here to ensure we do not change our behavior unintentionally
// With internal versions, tt means "if object is there with version X, update it or explode. If it is not there, index.
client().prepareUpdate(indexOrAlias(), "type", "3")
.setScript(new Script(ScriptType.INLINE, "put_values", "", Collections.singletonMap("text", "v2")))
.setVersion(10).setUpsert("{ \"text\": \"v0\" }", XContentType.JSON).get();
get = get("test", "type", "3");
assertThat(get.getVersion(), equalTo(1L));
assertThat((String) get.getSource().get("text"), equalTo("v0"));
// retry on conflict is rejected:
assertThrows(client().prepareUpdate(indexOrAlias(), "type", "1").setVersion(10).setRetryOnConflict(5), ActionRequestValidationException.class);
}
public void testIndexAutoCreation() throws Exception {
UpdateResponse updateResponse = client().prepareUpdate("test", "type1", "1")
.setUpsert(XContentFactory.jsonBuilder().startObject().field("bar", "baz").endObject())