Address code review comments

Signed-off-by: Daniel Widdis <[email protected]>
opensearch-project · Jul 19, 2024 · 2b07604 · 2b07604
1 parent 61badf8
commit 2b07604
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 8 deletions.
diff --git a/...ne-common/src/main/java/org/opensearch/search/pipeline/common/SplitResponseProcessor.java b/...ne-common/src/main/java/org/opensearch/search/pipeline/common/SplitResponseProcessor.java
@@ -22,11 +22,10 @@
 import org.opensearch.search.pipeline.Processor;
 import org.opensearch.search.pipeline.SearchResponseProcessor;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 /**
  * Processor that sorts an array of items.
@@ -117,7 +116,7 @@ public SearchResponse processResponse(SearchRequest request, SearchResponse resp
                     throw new IllegalArgumentException("field [" + splitField + "] is not a string, cannot split");
                 }
                 String[] strings = ((String) val).split(separator, preserveTrailing ? -1 : 0);
-                List<Object> splitList = Stream.of(strings).collect(Collectors.toList());
+                List<Object> splitList = List.copyOf(Arrays.asList(strings));
                 hit.setDocumentField(targetField, new DocumentField(targetField, splitList));
             }
             if (hit.hasSource()) {
@@ -133,7 +132,7 @@ public SearchResponse processResponse(SearchRequest request, SearchResponse resp
                     Object val = sourceAsMap.get(splitField);
                     if (val instanceof String) {
                         String[] strings = ((String) val).split(separator, preserveTrailing ? -1 : 0);
-                        List<Object> splitList = Stream.of(strings).collect(Collectors.toList());
+                        List<Object> splitList = List.copyOf(Arrays.asList(strings));
                         sourceAsMap.put(targetField, splitList);
                     }
                     XContentBuilder builder = XContentBuilder.builder(typeAndSourceMap.v1().xContent());
@@ -156,10 +155,10 @@ public SplitResponseProcessor create(
             Map<String, Object> config,
             PipelineContext pipelineContext
         ) {
-            String splitField = ConfigurationUtils.readStringProperty(TYPE, tag, config, "field");
-            String separator = ConfigurationUtils.readStringProperty(TYPE, tag, config, "separator");
-            boolean preserveTrailing = ConfigurationUtils.readBooleanProperty(TYPE, tag, config, "preserve_trailing", false);
-            String targetField = ConfigurationUtils.readStringProperty(TYPE, tag, config, "target_field", splitField);
+            String splitField = ConfigurationUtils.readStringProperty(TYPE, tag, config, SPLIT_FIELD);
+            String separator = ConfigurationUtils.readStringProperty(TYPE, tag, config, SEPARATOR);
+            boolean preserveTrailing = ConfigurationUtils.readBooleanProperty(TYPE, tag, config, PRESERVE_TRAILING, false);
+            String targetField = ConfigurationUtils.readStringProperty(TYPE, tag, config, TARGET_FIELD, splitField);
             return new SplitResponseProcessor(tag, description, ignoreFailure, splitField, separator, preserveTrailing, targetField);
         }
     }

diff --git a/...mmon/src/test/java/org/opensearch/search/pipeline/common/SplitResponseProcessorTests.java b/...mmon/src/test/java/org/opensearch/search/pipeline/common/SplitResponseProcessorTests.java
@@ -32,6 +32,7 @@ public class SplitResponseProcessorTests extends OpenSearchTestCase {
 
     private static final String NO_TRAILING = "one,two,three";
     private static final String TRAILING = "alpha,beta,gamma,";
+    private static final String REGEX_DELIM = "one1two2three";
 
     private SearchRequest createDummyRequest() {
         QueryBuilder query = new TermQueryBuilder("field", "value");
@@ -60,6 +61,20 @@ private SearchResponse createTestResponse() {
         return new SearchResponse(searchResponseSections, null, 1, 1, 0, 10, null, null);
     }
 
+    private SearchResponse createTestResponseRegex() {
+        SearchHit[] hits = new SearchHit[1];
+
+        Map<String, DocumentField> dsvMap = new HashMap<>();
+        dsvMap.put("dsv", new DocumentField("dsv", List.of(REGEX_DELIM)));
+        hits[0] = new SearchHit(0, "doc 1", dsvMap, Collections.emptyMap());
+        hits[0].sourceRef(new BytesArray("{ \"dsv\" : \"" + REGEX_DELIM + "\" }"));
+        hits[0].score(1f);
+
+        SearchHits searchHits = new SearchHits(hits, new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1);
+        SearchResponseSections searchResponseSections = new SearchResponseSections(searchHits, null, null, false, false, null, 0);
+        return new SearchResponse(searchResponseSections, null, 1, 1, 0, 10, null, null);
+    }
+
     private SearchResponse createTestResponseNullField() {
         SearchHit[] hits = new SearchHit[1];
 
@@ -122,6 +137,22 @@ public void testSplitResponse() throws Exception {
         assertNull(splitResponse.getHits().getHits()[1].getSourceAsMap());
     }
 
+    public void testSplitResponseRegex() throws Exception {
+        SearchRequest request = createDummyRequest();
+
+        SplitResponseProcessor splitResponseProcessor = new SplitResponseProcessor(null, null, false, "dsv", "\\d", false, "split");
+        SearchResponse response = createTestResponseRegex();
+        SearchResponse splitResponse = splitResponseProcessor.processResponse(request, response);
+
+        assertEquals(response.getHits(), splitResponse.getHits());
+
+        assertEquals(REGEX_DELIM, splitResponse.getHits().getHits()[0].field("dsv").getValue());
+        assertEquals(List.of("one", "two", "three"), splitResponse.getHits().getHits()[0].field("split").getValues());
+        Map<String, Object> map = splitResponse.getHits().getHits()[0].getSourceAsMap();
+        assertNotNull(map);
+        assertEquals(List.of("one", "two", "three"), map.get("split"));
+    }
+
     public void testSplitResponseSameField() throws Exception {
         SearchRequest request = createDummyRequest();