From 2b076044eedb673a32133764c5b8ec7d3ea96b84 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 19 Jul 2024 13:35:41 -0700 Subject: [PATCH] Address code review comments Signed-off-by: Daniel Widdis --- .../common/SplitResponseProcessor.java | 15 +++++---- .../common/SplitResponseProcessorTests.java | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/modules/search-pipeline-common/src/main/java/org/opensearch/search/pipeline/common/SplitResponseProcessor.java b/modules/search-pipeline-common/src/main/java/org/opensearch/search/pipeline/common/SplitResponseProcessor.java index 6bbf7386622a4..bd9ecac67dae0 100644 --- a/modules/search-pipeline-common/src/main/java/org/opensearch/search/pipeline/common/SplitResponseProcessor.java +++ b/modules/search-pipeline-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 splitList = Stream.of(strings).collect(Collectors.toList()); + List 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 splitList = Stream.of(strings).collect(Collectors.toList()); + List 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 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/modules/search-pipeline-common/src/test/java/org/opensearch/search/pipeline/common/SplitResponseProcessorTests.java b/modules/search-pipeline-common/src/test/java/org/opensearch/search/pipeline/common/SplitResponseProcessorTests.java index 394f4d696befd..fcbc8ccf43cff 100644 --- a/modules/search-pipeline-common/src/test/java/org/opensearch/search/pipeline/common/SplitResponseProcessorTests.java +++ b/modules/search-pipeline-common/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 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 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();