From d6bc17fee5f60dbdd3ef97897c5bf28050719cec Mon Sep 17 00:00:00 2001 From: Matt Weber Date: Sat, 16 Jun 2012 19:51:44 -0700 Subject: [PATCH] Partial update without script Allow the use of "doc" as the update source when a script is not specified. New fields are added, existing fields are overwritten, and maps are merged recursively. --- .../action/index/IndexRequest.java | 5 + .../action/update/TransportUpdateAction.java | 96 +++++++++++----- .../action/update/UpdateRequest.java | 104 +++++++++++++++++- .../action/update/UpdateRequestBuilder.java | 56 ++++++++++ .../rest/action/update/RestUpdateAction.java | 11 ++ .../test/integration/update/UpdateTests.java | 54 +++++++++ 6 files changed, 295 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/src/main/java/org/elasticsearch/action/index/IndexRequest.java index b75d1b6e37b..8e00672ca3d 100644 --- a/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.mapper.internal.TimestampFieldMapper; @@ -333,6 +334,10 @@ public class IndexRequest extends ShardReplicationOperationRequest { return new BytesHolder(underlyingSource(), underlyingSourceOffset(), underlyingSourceLength()); } + public Map underlyingSourceAsMap() { + return XContentHelper.convertToMap(underlyingSource(), underlyingSourceOffset(), underlyingSourceLength(), false).v2(); + } + public byte[] underlyingSource() { if (sourceUnsafe) { source(); diff --git a/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java b/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java index c826c7efcbe..26fef66c26c 100644 --- a/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -216,37 +216,59 @@ public class TransportUpdateAction extends TransportInstanceSingleOperationActio return; } - Tuple> sourceAndContent = XContentHelper.convertToMap(getResult.internalSourceRef().bytes(), getResult.internalSourceRef().offset(), getResult.internalSourceRef().length(), true); - Map ctx = new HashMap(2); - ctx.put("_source", sourceAndContent.v2()); - - try { - ExecutableScript script = scriptService.executable(request.scriptLang, request.script, request.scriptParams); - script.setNextVar("ctx", ctx); - script.run(); - // we need to unwrap the ctx... - ctx = (Map) script.unwrap(ctx); - } catch (Exception e) { - throw new ElasticSearchIllegalArgumentException("failed to execute script", e); - } - - String operation = (String) ctx.get("op"); - String timestamp = (String) ctx.get("_timestamp"); + Tuple> sourceAndContent = XContentHelper.convertToMap(getResult.internalSourceRef().bytes(), getResult.internalSourceRef().offset(), getResult.internalSourceRef().length(), true); + String operation = null; + String timestamp = null; Long ttl = null; - Object fetchedTTL = ctx.get("_ttl"); - if (fetchedTTL != null) { - if (fetchedTTL instanceof Number) { - ttl = ((Number) fetchedTTL).longValue(); - } else { - ttl = TimeValue.parseTimeValue((String) fetchedTTL, null).millis(); - } - } - final Map updatedSourceAsMap = (Map) ctx.get("_source"); + Object fetchedTTL = null; + final Map updatedSourceAsMap; final XContentType updateSourceContentType = sourceAndContent.v1(); - - // apply script to update the source String routing = getResult.fields().containsKey(RoutingFieldMapper.NAME) ? getResult.field(RoutingFieldMapper.NAME).value().toString() : null; String parent = getResult.fields().containsKey(ParentFieldMapper.NAME) ? getResult.field(ParentFieldMapper.NAME).value().toString() : null; + + if (request.script() == null && request.doc() != null) { + IndexRequest indexRequest = request.doc(); + updatedSourceAsMap = sourceAndContent.v2(); + if (indexRequest.ttl() > 0) { + ttl = indexRequest.ttl(); + } + timestamp = indexRequest.timestamp(); + if (indexRequest.routing() != null) { + routing = indexRequest.routing(); + } + if (indexRequest.parent() != null) { + parent = indexRequest.parent(); + } + updateSource(updatedSourceAsMap, indexRequest.underlyingSourceAsMap()); + } else { + Map ctx = new HashMap(2); + ctx.put("_source", sourceAndContent.v2()); + + try { + ExecutableScript script = scriptService.executable(request.scriptLang, request.script, request.scriptParams); + script.setNextVar("ctx", ctx); + script.run(); + // we need to unwrap the ctx... + ctx = (Map) script.unwrap(ctx); + } catch (Exception e) { + throw new ElasticSearchIllegalArgumentException("failed to execute script", e); + } + + operation = (String) ctx.get("op"); + timestamp = (String) ctx.get("_timestamp"); + fetchedTTL = ctx.get("_ttl"); + if (fetchedTTL != null) { + if (fetchedTTL instanceof Number) { + ttl = ((Number) fetchedTTL).longValue(); + } else { + ttl = TimeValue.parseTimeValue((String) fetchedTTL, null).millis(); + } + } + + updatedSourceAsMap = (Map) ctx.get("_source"); + } + + // apply script to update the source // No TTL has been given in the update script so we keep previous TTL value if there is one if (ttl == null) { ttl = getResult.fields().containsKey(TTLFieldMapper.NAME) ? (Long) getResult.field(TTLFieldMapper.NAME).value() : null; @@ -366,4 +388,24 @@ public class TransportUpdateAction extends TransportInstanceSingleOperationActio return new GetResult(request.index(), request.type(), request.id(), version, true, sourceRequested ? sourceAsBytes : null, fields); } + + /** + * Updates the source with the specified changes. Maps are updated recursively. + */ + private void updateSource(Map source, Map changes) { + for (Map.Entry changesEntry : changes.entrySet()) { + if (!source.containsKey(changesEntry.getKey())) { + // safe to copy, change does not exist in source + source.put(changesEntry.getKey(), changesEntry.getValue()); + } else { + if (source.get(changesEntry.getKey()) instanceof Map && changesEntry.getValue() instanceof Map) { + // recursive merge maps + updateSource((Map) source.get(changesEntry.getKey()), (Map) changesEntry.getValue()); + } else { + // update the field + source.put(changesEntry.getKey(), changesEntry.getValue()); + } + } + } + } } diff --git a/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/src/main/java/org/elasticsearch/action/update/UpdateRequest.java index fcb3b251278..97769606b24 100644 --- a/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/src/main/java/org/elasticsearch/action/update/UpdateRequest.java @@ -48,6 +48,7 @@ public class UpdateRequest extends InstanceShardOperationRequest { @Nullable private String routing; + @Nullable String script; @Nullable String scriptLang; @@ -67,6 +68,9 @@ public class UpdateRequest extends InstanceShardOperationRequest { private IndexRequest upsertRequest; + @Nullable + private IndexRequest doc; + UpdateRequest() { } @@ -86,8 +90,8 @@ public class UpdateRequest extends InstanceShardOperationRequest { if (id == null) { validationException = addValidationError("id is missing", validationException); } - if (script == null) { - validationException = addValidationError("script is missing", validationException); + if (script == null && doc == null) { + validationException = addValidationError("script or doc is missing", validationException); } return validationException; } @@ -345,6 +349,73 @@ public class UpdateRequest extends InstanceShardOperationRequest { return this; } + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(IndexRequest doc) { + this.doc = doc; + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(XContentBuilder source) { + safeDoc().source(source); + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(Map source) { + safeDoc().source(source); + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(Map source, XContentType contentType) { + safeDoc().source(source, contentType); + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(String source) { + safeDoc().source(source); + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(byte[] source) { + safeDoc().source(source); + return this; + } + + /** + * Sets the doc to use for updates when a script is not specified. + */ + public UpdateRequest doc(byte[] source, int offset, int length) { + safeDoc().source(source, offset, length); + return this; + } + + public IndexRequest doc() { + return this.doc; + } + + private IndexRequest safeDoc() { + if (doc == null) { + doc = new IndexRequest(); + } + return doc; + } + /** * Sets the index request to be used if the document does not exists. Otherwise, a {@link org.elasticsearch.index.engine.DocumentMissingException} * is thrown. @@ -442,6 +513,10 @@ public class UpdateRequest extends InstanceShardOperationRequest { XContentBuilder builder = XContentFactory.contentBuilder(xContentType); builder.copyCurrentStructure(parser); safeUpsertRequest().source(builder); + } else if ("doc".equals(currentFieldName)) { + XContentBuilder docBuilder = XContentFactory.contentBuilder(xContentType); + docBuilder.copyCurrentStructure(parser); + safeDoc().source(docBuilder); } } return this; @@ -457,7 +532,9 @@ public class UpdateRequest extends InstanceShardOperationRequest { if (in.readBoolean()) { routing = in.readUTF(); } - script = in.readUTF(); + if (in.readBoolean()) { + script = in.readUTF(); + } if (in.readBoolean()) { scriptLang = in.readUTF(); } @@ -467,6 +544,10 @@ public class UpdateRequest extends InstanceShardOperationRequest { percolate = in.readUTF(); } refresh = in.readBoolean(); + if (in.readBoolean()) { + doc = new IndexRequest(); + doc.readFrom(in); + } int size = in.readInt(); if (size >= 0) { fields = new String[size]; @@ -493,7 +574,12 @@ public class UpdateRequest extends InstanceShardOperationRequest { out.writeBoolean(true); out.writeUTF(routing); } - out.writeUTF(script); + if (script == null) { + out.writeBoolean(false); + } else { + out.writeBoolean(true); + out.writeUTF(script); + } if (scriptLang == null) { out.writeBoolean(false); } else { @@ -509,6 +595,16 @@ public class UpdateRequest extends InstanceShardOperationRequest { out.writeUTF(percolate); } out.writeBoolean(refresh); + if (doc == null) { + out.writeBoolean(false); + } else { + out.writeBoolean(true); + // make sure the basics are set + doc.index(index); + doc.type(type); + doc.id(id); + doc.writeTo(out); + } if (fields == null) { out.writeInt(-1); } else { diff --git a/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java b/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java index 3cd06e54f77..b7fe8ec5f2a 100644 --- a/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java +++ b/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java @@ -183,6 +183,62 @@ public class UpdateRequestBuilder extends BaseRequestBuilder doc = request.doc().underlyingSourceAsMap(); + assertThat(doc.get("field1").toString(), equalTo("value1")); + assertThat(((Map) doc.get("compound")).get("field2").toString(), equalTo("value2")); } @Test @@ -272,5 +282,49 @@ public class UpdateTests extends AbstractNodesTests { assertThat(updateResponse.getResult(), notNullValue()); assertThat(updateResponse.getResult().sourceRef(), notNullValue()); assertThat(updateResponse.getResult().field("field").value(), notNullValue()); + + // check updates without script + // add new field + client.prepareIndex("test", "type1", "1").setSource("field", 1).execute().actionGet(); + updateResponse = client.prepareUpdate("test", "type1", "1").setDoc(XContentFactory.jsonBuilder().startObject().field("field2", 2).endObject()).execute().actionGet(); + for (int i = 0; i < 5; i++) { + getResponse = client.prepareGet("test", "type1", "1").execute().actionGet(); + assertThat(getResponse.sourceAsMap().get("field").toString(), equalTo("1")); + assertThat(getResponse.sourceAsMap().get("field2").toString(), equalTo("2")); + } + + // change existing field + updateResponse = client.prepareUpdate("test", "type1", "1").setDoc(XContentFactory.jsonBuilder().startObject().field("field", 3).endObject()).execute().actionGet(); + for (int i = 0; i < 5; i++) { + getResponse = client.prepareGet("test", "type1", "1").execute().actionGet(); + assertThat(getResponse.sourceAsMap().get("field").toString(), equalTo("3")); + assertThat(getResponse.sourceAsMap().get("field2").toString(), equalTo("2")); + } + + // recursive map + Map testMap = new HashMap(); + Map testMap2 = new HashMap(); + Map testMap3 = new HashMap(); + testMap3.put("commonkey", testMap); + testMap3.put("map3", 5); + testMap2.put("map2", 6); + testMap.put("commonkey", testMap2); + testMap.put("map1", 8); + + client.prepareIndex("test", "type1", "1").setSource("map", testMap).execute().actionGet(); + updateResponse = client.prepareUpdate("test", "type1", "1").setDoc(XContentFactory.jsonBuilder().startObject().field("map", testMap3).endObject()).execute().actionGet(); + for (int i = 0; i < 5; i++) { + getResponse = client.prepareGet("test", "type1", "1").execute().actionGet(); + Map map1 = (Map) getResponse.sourceAsMap().get("map"); + assertThat(map1.size(), equalTo(3)); + assertThat(map1.containsKey("map1"), equalTo(true)); + assertThat(map1.containsKey("map3"), equalTo(true)); + assertThat(map1.containsKey("commonkey"), equalTo(true)); + Map map2 = (Map) map1.get("commonkey"); + assertThat(map2.size(), equalTo(3)); + assertThat(map2.containsKey("map1"), equalTo(true)); + assertThat(map2.containsKey("map2"), equalTo(true)); + assertThat(map2.containsKey("commonkey"), equalTo(true)); + } } }