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

Remove sourceModified flag from IngestDocument 

If one is using the ingest plugin and providing a pipeline id with the request, the chance that the source is going to be modified is 99%. We shouldn't worry about keeping track of whether something changed. That seemed useful at first so we can save the resources for setting back the source (map to bytes) when not needed. Also, we are trying to unify metadata fields and source in the same map and that is going to complicate how we keep track of changes that happen in the source only. Best solution is to remove the flag.
This commit is contained in:
javanna 2015-12-09 10:50:10 +01:00 committed by Luca Cavanna
parent b0d7d604ff
commit 6b7446beb9
4 changed files with 1 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
plugins/ingest/src/main/java/org/elasticsearch/ingest/IngestDocument.java
View File

@ -36,8 +36,6 @@ public final class IngestDocument {
private final Map<String, Object> source;
private final Map<String, String> ingestMetadata;
private boolean sourceModified = false;
public IngestDocument(String index, String type, String id, String routing, String parent, String timestamp, String ttl, Map<String, Object> source) {
this.esMetadata = new HashMap<>();
this.esMetadata.put(MetaData.INDEX.getFieldName(), index);
@ -190,7 +188,6 @@ public final class IngestDocument {
Map<String, Object> map = (Map<String, Object>) context;
if (map.containsKey(leafKey)) {
map.remove(leafKey);
this.sourceModified = true;
return;
}
throw new IllegalArgumentException("field [" + leafKey + "] not present as part of path [" + path + "]");
@ -208,7 +205,6 @@ public final class IngestDocument {
throw new IllegalArgumentException("[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]");
}
list.remove(index);
this.sourceModified = true;
return;
}
@ -292,7 +288,6 @@ public final class IngestDocument {
} else {
HashMap<Object, Object> newMap = new HashMap<>();
map.put(pathElement, newMap);
sourceModified = true;
context = newMap;
}
} else if (context instanceof List) {
@ -327,13 +322,11 @@ public final class IngestDocument {
@SuppressWarnings("unchecked")
List<Object> list = (List<Object>) object;
list.add(value);
sourceModified = true;
return;
}
}
}
map.put(leafKey, value);
sourceModified = true;
} else if (context instanceof List) {
@SuppressWarnings("unchecked")
List<Object> list = (List<Object>) context;
@ -347,7 +340,6 @@ public final class IngestDocument {
throw new IllegalArgumentException("[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]");
}
list.set(index, value);
this.sourceModified = true;
} else {
throw new IllegalArgumentException("cannot set [" + leafKey + "] with parent object of type [" + context.getClass().getName() + "] as part of path [" + path + "]");
}
@ -386,10 +378,6 @@ public final class IngestDocument {
return source;
}
public boolean isSourceModified() {
return sourceModified;
}
static Object deepCopy(Object value) {
if (value instanceof Map) {
@SuppressWarnings("unchecked")

4
plugins/ingest/src/main/java/org/elasticsearch/plugin/ingest/PipelineExecutionService.java
View File

@ -62,9 +62,7 @@ public class PipelineExecutionService {
IngestDocument ingestDocument = new IngestDocument(index, type, id, routing, parent, timestamp, ttl, sourceAsMap);
try {
pipeline.execute(ingestDocument);
if (ingestDocument.isSourceModified()) {
indexRequest.source(ingestDocument.getSource());
}
indexRequest.source(ingestDocument.getSource());
indexRequest.index(ingestDocument.getEsMetadata(IngestDocument.MetaData.INDEX));
indexRequest.type(ingestDocument.getEsMetadata(IngestDocument.MetaData.TYPE));
indexRequest.id(ingestDocument.getEsMetadata(IngestDocument.MetaData.ID));

2
plugins/ingest/src/main/java/org/elasticsearch/plugin/ingest/transport/simulate/WriteableIngestDocument.java
View File

@ -74,7 +74,6 @@ final class WriteableIngestDocument implements Writeable<WriteableIngestDocument
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(Fields.DOCUMENT);
builder.field(Fields.MODIFIED, ingestDocument.isSourceModified());
for (Map.Entry<String, String> esMetadata : ingestDocument.getEsMetadata().entrySet()) {
builder.field(esMetadata.getKey(), esMetadata.getValue());
}
@ -112,7 +111,6 @@ final class WriteableIngestDocument implements Writeable<WriteableIngestDocument
static final class Fields {
static final XContentBuilderString DOCUMENT = new XContentBuilderString("doc");
static final XContentBuilderString MODIFIED = new XContentBuilderString("modified");
static final XContentBuilderString SOURCE = new XContentBuilderString("_source");
static final XContentBuilderString INGEST = new XContentBuilderString("_ingest");
}

29
plugins/ingest/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java
View File

@ -49,7 +49,6 @@ public class IngestDocumentTests extends ESTestCase {
list.add(null);
document.put("list", list);
ingestDocument = new IngestDocument("index", "type", "id", null, null, null, null, document);
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
public void testSimpleGetFieldValue() {
@ -205,14 +204,12 @@ public class IngestDocumentTests extends ESTestCase {
public void testSimpleSetFieldValue() {
ingestDocument.setFieldValue("new_field", "foo");
assertThat(ingestDocument.getSource().get("new_field"), equalTo("foo"));
assertThat(ingestDocument.isSourceModified(), equalTo(true));
}
public void testSetFieldValueNullValue() {
ingestDocument.setFieldValue("new_field", null);
assertThat(ingestDocument.getSource().containsKey("new_field"), equalTo(true));
assertThat(ingestDocument.getSource().get("new_field"), nullValue());
assertThat(ingestDocument.isSourceModified(), equalTo(true));
}
@SuppressWarnings("unchecked")
@ -227,7 +224,6 @@ public class IngestDocumentTests extends ESTestCase {
assertThat(c.get("d"), instanceOf(String.class));
String d = (String) c.get("d");
assertThat(d, equalTo("foo"));
assertThat(ingestDocument.isSourceModified(), equalTo(true));
}
public void testSetFieldValueOnExistingField() {
@ -243,7 +239,6 @@ public class IngestDocumentTests extends ESTestCase {
assertThat(innerMap.get("new"), instanceOf(String.class));
String value = (String) innerMap.get("new");
assertThat(value, equalTo("bar"));
assertThat(ingestDocument.isSourceModified(), equalTo(true));
}
public void testSetFieldValueOnExistingParentTypeMismatch() {
@ -252,7 +247,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("add field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("cannot set [new] with parent object of type [java.lang.String] as part of path [fizz.buzz.new]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -262,7 +256,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("add field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("cannot set [test] with null parent as part of path [fizz.foo_null.test]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -272,13 +265,11 @@ public class IngestDocumentTests extends ESTestCase {
fail("add field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("path cannot be null nor empty"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
public void testListSetFieldValueNoIndexProvided() {
ingestDocument.setFieldValue("list", "value");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
Object object = ingestDocument.getSource().get("list");
assertThat(object, instanceOf(String.class));
assertThat(object, equalTo("value"));
@ -286,7 +277,6 @@ public class IngestDocumentTests extends ESTestCase {
public void testListAppendFieldValue() {
ingestDocument.appendFieldValue("list", "new_value");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
Object object = ingestDocument.getSource().get("list");
assertThat(object, instanceOf(List.class));
@SuppressWarnings("unchecked")
@ -299,7 +289,6 @@ public class IngestDocumentTests extends ESTestCase {
public void testListSetFieldValueIndexProvided() {
ingestDocument.setFieldValue("list.1", "value");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
Object object = ingestDocument.getSource().get("list");
assertThat(object, instanceOf(List.class));
@SuppressWarnings("unchecked")
@ -311,7 +300,6 @@ public class IngestDocumentTests extends ESTestCase {
public void testSetFieldValueListAsPartOfPath() {
ingestDocument.setFieldValue("list.0.field", "new_value");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
Object object = ingestDocument.getSource().get("list");
assertThat(object, instanceOf(List.class));
@SuppressWarnings("unchecked")
@ -326,14 +314,12 @@ public class IngestDocumentTests extends ESTestCase {
ingestDocument.setFieldValue("list.test", "value");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[test] is not an integer, cannot be used as an index as part of path [list.test]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
try {
ingestDocument.setFieldValue("list.test.field", "new_value");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[test] is not an integer, cannot be used as an index as part of path [list.test.field]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -342,14 +328,12 @@ public class IngestDocumentTests extends ESTestCase {
ingestDocument.setFieldValue("list.10", "value");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[10] is out of bounds for array with length [2] as part of path [list.10]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
try {
ingestDocument.setFieldValue("list.10.field", "value");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[10] is out of bounds for array with length [2] as part of path [list.10.field]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -359,13 +343,11 @@ public class IngestDocumentTests extends ESTestCase {
fail("add field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("path cannot be null nor empty"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
public void testRemoveField() {
ingestDocument.removeField("foo");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
assertThat(ingestDocument.getSource().size(), equalTo(3));
assertThat(ingestDocument.getSource().containsKey("foo"), equalTo(false));
}
@ -383,13 +365,11 @@ public class IngestDocumentTests extends ESTestCase {
assertThat(map.size(), equalTo(1));
assertThat(ingestDocument.getSource().size(), equalTo(4));
assertThat(ingestDocument.getSource().containsKey("fizz"), equalTo(true));
assertThat(ingestDocument.isSourceModified(), equalTo(true));
ingestDocument.removeField("fizz.1");
assertThat(map.size(), equalTo(0));
assertThat(ingestDocument.getSource().size(), equalTo(4));
assertThat(ingestDocument.getSource().containsKey("fizz"), equalTo(true));
assertThat(ingestDocument.isSourceModified(), equalTo(true));
}
public void testRemoveNonExistingField() {
@ -398,7 +378,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("remove field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("field [does_not_exist] not present as part of path [does_not_exist]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -408,13 +387,11 @@ public class IngestDocumentTests extends ESTestCase {
fail("remove field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("cannot resolve [foo] from object of type [java.lang.String] as part of path [foo.foo.bar]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
public void testListRemoveField() {
ingestDocument.removeField("list.0.field");
assertThat(ingestDocument.isSourceModified(), equalTo(true));
assertThat(ingestDocument.getSource().size(), equalTo(4));
assertThat(ingestDocument.getSource().containsKey("list"), equalTo(true));
Object object = ingestDocument.getSource().get("list");
@ -438,7 +415,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("get field value should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("cannot remove [not_there] from null as part of path [fizz.foo_null.not_there]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -447,7 +423,6 @@ public class IngestDocumentTests extends ESTestCase {
ingestDocument.removeField("fizz.1.bar");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("cannot remove [bar] from object of type [java.lang.String] as part of path [fizz.1.bar]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -456,7 +431,6 @@ public class IngestDocumentTests extends ESTestCase {
ingestDocument.removeField("list.test");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[test] is not an integer, cannot be used as an index as part of path [list.test]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -465,7 +439,6 @@ public class IngestDocumentTests extends ESTestCase {
ingestDocument.removeField("list.10");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[10] is out of bounds for array with length [2] as part of path [list.10]"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -475,7 +448,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("remove field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("path cannot be null nor empty"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}
@ -485,7 +457,6 @@ public class IngestDocumentTests extends ESTestCase {
fail("remove field should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("path cannot be null nor empty"));
assertThat(ingestDocument.isSourceModified(), equalTo(false));
}
}