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

Make rename processor less error prone 

Rename processor now checks whether the field to rename exists and throws exception if it doesn't. It also checks that the new field to rename to doesn't exist yet, and throws exception otherwise. Also we make sure that the rename operation is atomic, otherwise things may break between the remove and the set and we'd leave the document in an inconsistent state.

Note that the requirement for the new field name to not exist simplifies the usecase for e.g. { "rename" : { "list.1": "list.2"} } as such a rename wouldn't be accepted if list is actually a list given that either list.2 already exists or the index is out of bounds for the existing list. If one really wants to replace an existing field, that field needs to be removed first through remove processor and then rename can be used.
This commit is contained in:
javanna 2015-12-01 19:23:44 +01:00 committed by Luca Cavanna
parent 15b6708a5d
commit 6c0510b01d
3 changed files with 128 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/plugins/ingest.asciidoc
View File

@ -33,7 +33,8 @@ Removes one or more existing fields. If a field doesn't exist, nothing will happ
--------------------------------------------------
==== Rename processor
Renames one or more existing fields. If a field doesn't exist, an exception will be thrown.
Renames one or more existing fields. If a field doesn't exist, an exception will be thrown. Also the new field
name must not exist.
[source,js]
--------------------------------------------------

20
plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/rename/RenameProcessor.java
View File

@ -46,13 +46,23 @@ public class RenameProcessor implements Processor {
@Override
public void execute(IngestDocument document) {
for(Map.Entry<String, String> entry : fields.entrySet()) {
if (document.hasField(entry.getKey())) {
if (document.hasField(entry.getKey()) == false) {
throw new IllegalArgumentException("field [" + entry.getKey() + "] doesn't exist");
String oldFieldName = entry.getKey();
if (document.hasField(oldFieldName) == false) {
throw new IllegalArgumentException("field [" + oldFieldName + "] doesn't exist");
}
String newFieldName = entry.getValue();
if (document.hasField(newFieldName)) {
throw new IllegalArgumentException("field [" + newFieldName + "] already exists");
}
Object oldValue = document.getFieldValue(entry.getKey(), Object.class);
document.removeField(entry.getKey());
document.setFieldValue(entry.getValue(), oldValue);
document.setFieldValue(newFieldName, oldValue);
try {
document.removeField(oldFieldName);
} catch (Exception e) {
//remove the new field if the removal of the old one failed
document.removeField(newFieldName);
throw e;
}
}
}

111
plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/rename/RenameProcessorTests.java
View File

@ -24,10 +24,10 @@ import org.elasticsearch.ingest.RandomDocumentPicks;
import org.elasticsearch.ingest.processor.Processor;
import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.util.*;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.nullValue;
public class RenameProcessorTests extends ESTestCase {
@ -56,11 +56,66 @@ public class RenameProcessorTests extends ESTestCase {
}
}
public void testRenameArrayElement() throws Exception {
Map<String, Object> document = new HashMap<>();
List<String> list = new ArrayList<>();
list.add("item1");
list.add("item2");
list.add("item3");
document.put("list", list);
List<Map<String, String>> one = new ArrayList<>();
one.add(Collections.singletonMap("one", "one"));
one.add(Collections.singletonMap("two", "two"));
document.put("one", one);
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
Processor processor = new RenameProcessor(Collections.singletonMap("list.0", "item"));
processor.execute(ingestDocument);
Object actualObject = ingestDocument.getSource().get("list");
assertThat(actualObject, instanceOf(List.class));
@SuppressWarnings("unchecked")
List<String> actualList = (List<String>) actualObject;
assertThat(actualList.size(), equalTo(2));
assertThat(actualList.get(0), equalTo("item2"));
assertThat(actualList.get(1), equalTo("item3"));
actualObject = ingestDocument.getSource().get("item");
assertThat(actualObject, instanceOf(String.class));
assertThat(actualObject, equalTo("item1"));
processor = new RenameProcessor(Collections.singletonMap("list.0", "list.3"));
try {
processor.execute(ingestDocument);
fail("processor execute should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("[3] is out of bounds for array with length [2] as part of path [list.3]"));
assertThat(actualList.size(), equalTo(2));
assertThat(actualList.get(0), equalTo("item2"));
assertThat(actualList.get(1), equalTo("item3"));
}
}
public void testRenameNonExistingField() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
Processor processor = new RenameProcessor(Collections.singletonMap(RandomDocumentPicks.randomFieldName(random()), RandomDocumentPicks.randomFieldName(random())));
String fieldName = RandomDocumentPicks.randomFieldName(random());
Processor processor = new RenameProcessor(Collections.singletonMap(fieldName, RandomDocumentPicks.randomFieldName(random())));
try {
processor.execute(ingestDocument);
assertThat(ingestDocument.getSource().size(), equalTo(0));
fail("processor execute should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("field [" + fieldName + "] doesn't exist"));
}
}
public void testRenameNewFieldAlreadyExists() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
String fieldName = RandomDocumentPicks.randomExistingFieldName(random(), ingestDocument);
Processor processor = new RenameProcessor(Collections.singletonMap(RandomDocumentPicks.randomExistingFieldName(random(), ingestDocument), fieldName));
try {
processor.execute(ingestDocument);
fail("processor execute should have failed");
} catch(IllegalArgumentException e) {
assertThat(e.getMessage(), equalTo("field [" + fieldName + "] already exists"));
}
}
public void testRenameExistingFieldNullValue() throws Exception {
@ -74,4 +129,54 @@ public class RenameProcessorTests extends ESTestCase {
assertThat(ingestDocument.hasField(newFieldName), equalTo(true));
assertThat(ingestDocument.getFieldValue(newFieldName, Object.class), nullValue());
}
public void testRenameAtomicOperationSetFails() throws Exception {
Map<String, Object> document = new HashMap<String, Object>() {
private static final long serialVersionUID = 362498820763181265L;
@Override
public Object put(String key, Object value) {
if (key.equals("new_field")) {
throw new UnsupportedOperationException();
}
return super.put(key, value);
}
};
document.put("list", Collections.singletonList("item"));
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
Processor processor = new RenameProcessor(Collections.singletonMap("list", "new_field"));
try {
processor.execute(ingestDocument);
fail("processor execute should have failed");
} catch(UnsupportedOperationException e) {
//the set failed, the old field has not been removed
assertThat(ingestDocument.getSource().containsKey("list"), equalTo(true));
assertThat(ingestDocument.getSource().containsKey("new_field"), equalTo(false));
}
}
public void testRenameAtomicOperationRemoveFails() throws Exception {
Map<String, Object> document = new HashMap<String, Object>() {
private static final long serialVersionUID = 362498820763181265L;
@Override
public Object remove(Object key) {
if (key.equals("list")) {
throw new UnsupportedOperationException();
}
return super.remove(key);
}
};
document.put("list", Collections.singletonList("item"));
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
Processor processor = new RenameProcessor(Collections.singletonMap("list", "new_field"));
try {
processor.execute(ingestDocument);
fail("processor execute should have failed");
} catch (UnsupportedOperationException e) {
//the set failed, the old field has not been removed
assertThat(ingestDocument.getSource().containsKey("list"), equalTo(true));
assertThat(ingestDocument.getSource().containsKey("new_field"), equalTo(false));
}
}
}