Skip to content

Commit

Permalink
fix: Sort Processor does not have proper behavior with targetField (#…
Browse files Browse the repository at this point in the history
…25237)

to specify a `targetField`. This results in some interesting behavior that was missed in the review.
This processor sorts in-place, so there is a side-effect in both the original field and the target field.
Another bug was that the targetField was not being set if the list being sorted was fewer than two elements.

The new behavior works like this: If targetField and fieldName are not the same, we copy the list.
  • Loading branch information
talevy committed Jun 15, 2017
1 parent 469194d commit b839a14
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.Processor;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -99,17 +100,15 @@ public void execute(IngestDocument document) {
throw new IllegalArgumentException("field [" + field + "] is null, cannot sort.");
}

if (list.size() <= 1) {
return;
}
List<? extends Comparable> copy = new ArrayList<>(list);

if (order.equals(SortOrder.ASCENDING)) {
Collections.sort(list);
Collections.sort(copy);
} else {
Collections.sort(list, Collections.reverseOrder());
Collections.sort(copy, Collections.reverseOrder());
}

document.setFieldValue(targetField, list);
document.setFieldValue(targetField, copy);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public void testSortNullValue() throws Exception {
}
}

public void testSortWithTargetField() throws Exception {
public void testDescendingSortWithTargetField() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
int numItems = randomIntBetween(1, 10);
List<String> fieldValue = new ArrayList<>(numItems);
Expand All @@ -285,18 +285,54 @@ public void testSortWithTargetField() throws Exception {
fieldValue.add(value);
expectedResult.add(value);
}

Collections.sort(expectedResult, Collections.reverseOrder());

String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, fieldValue);
String targetFieldName = RandomDocumentPicks.randomFieldName(random());
Processor processor = new SortProcessor(randomAlphaOfLength(10), fieldName,
SortOrder.DESCENDING, targetFieldName);
processor.execute(ingestDocument);
assertEquals(ingestDocument.getFieldValue(targetFieldName, List.class), expectedResult);
}

public void testAscendingSortWithTargetField() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
int numItems = randomIntBetween(1, 10);
List<String> fieldValue = new ArrayList<>(numItems);
List<String> expectedResult = new ArrayList<>(numItems);
for (int j = 0; j < numItems; j++) {
String value = randomAlphaOfLengthBetween(1, 10);
fieldValue.add(value);
expectedResult.add(value);
}

Collections.sort(expectedResult);

String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, fieldValue);
String targetFieldName = RandomDocumentPicks.randomFieldName(random());
Processor processor = new SortProcessor(randomAlphaOfLength(10), fieldName,
SortOrder.ASCENDING, targetFieldName);
processor.execute(ingestDocument);
assertEquals(ingestDocument.getFieldValue(targetFieldName, List.class), expectedResult);
}

public void testSortWithTargetFieldLeavesOriginalUntouched() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
List<Integer> fieldValue = Arrays.asList(1, 5, 4);
List<Integer> expectedResult = new ArrayList<>(fieldValue);
Collections.sort(expectedResult);

SortOrder order = randomBoolean() ? SortOrder.ASCENDING : SortOrder.DESCENDING;
if (order.equals(SortOrder.DESCENDING)) {
Collections.reverse(expectedResult);
}

String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, fieldValue);
String targetFieldName = RandomDocumentPicks.randomFieldName(random());
String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, new ArrayList<>(fieldValue));
String targetFieldName = fieldName + "foo";
Processor processor = new SortProcessor(randomAlphaOfLength(10), fieldName, order, targetFieldName);
processor.execute(ingestDocument);
assertEquals(ingestDocument.getFieldValue(targetFieldName, List.class), expectedResult);
assertEquals(ingestDocument.getFieldValue(fieldName, List.class), fieldValue);
}

}

0 comments on commit b839a14

Please sign in to comment.