From 139389c430f29ed9e4f494ab727c0044518acbed Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 12 Feb 2024 14:03:59 -0800 Subject: [PATCH 1/3] Add support for deep copying SearchRequest Signed-off-by: Chenyang Ji --- .../action/search/SearchRequest.java | 22 +++++++++++++++++++ .../action/search/SearchRequestTests.java | 7 ++++++ 2 files changed, 29 insertions(+) diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index f3d9f77e2394c..4a2e463d5418a 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -40,6 +40,7 @@ import org.opensearch.action.support.IndicesOptions; import org.opensearch.common.Nullable; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; @@ -161,6 +162,27 @@ public SearchRequest(String[] indices, SearchSourceBuilder source) { this.source = source; } + /** + * Deep clone a SearchRequest + * + * @return a copy of the current SearchRequest + */ + @Override + public SearchRequest clone() { + try (BytesStreamOutput out = new BytesStreamOutput()) { + try { + this.writeTo(out); + } catch (IOException e) { + throw new IllegalArgumentException(e); + } + try (StreamInput in = out.bytes().streamInput()) { + return new SearchRequest(in); + } catch (IOException e) { + throw new IllegalArgumentException(e); + } + } + } + /** * Creates a new sub-search request starting from the original search request that is provided. * For internal use only, allows to fork a search request into multiple search requests that will be executed independently. diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java index 9ee314e77ca7e..41d77ed656772 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java @@ -76,6 +76,13 @@ protected SearchRequest createSearchRequest() throws IOException { ); } + public void testClone() { + SearchRequest searchRequest = new SearchRequest(); + SearchRequest clonedRequest = searchRequest.clone(); + assertEquals(searchRequest.hashCode(), clonedRequest.hashCode()); + assertNotSame(searchRequest, clonedRequest); + } + public void testWithLocalReduction() { expectThrows(NullPointerException.class, () -> SearchRequest.subSearchRequest(null, Strings.EMPTY_ARRAY, "", 0, randomBoolean())); SearchRequest request = new SearchRequest(); From 5abf95503317eb93e1d9be7d2dbb4b1c65a490ea Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Wed, 10 Apr 2024 15:08:49 -0700 Subject: [PATCH 2/3] add unit test for complex data structures Signed-off-by: Chenyang Ji --- .../opensearch/action/search/SearchRequestTests.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java index 41d77ed656772..6468f69cc82e8 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java @@ -46,6 +46,7 @@ import org.opensearch.search.Scroll; import org.opensearch.search.builder.PointInTimeBuilder; import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.search.fetch.subphase.FetchSourceContext; import org.opensearch.search.rescore.QueryRescorerBuilder; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; @@ -76,11 +77,20 @@ protected SearchRequest createSearchRequest() throws IOException { ); } - public void testClone() { + public void testClone() throws IOException { SearchRequest searchRequest = new SearchRequest(); SearchRequest clonedRequest = searchRequest.clone(); assertEquals(searchRequest.hashCode(), clonedRequest.hashCode()); assertNotSame(searchRequest, clonedRequest); + + SearchSourceBuilder source = new SearchSourceBuilder() + .fetchSource(new FetchSourceContext(true, new String[] { "field1.*" }, new String[] { "field2.*" })); + SearchRequest complexSearchRequest = createSearchRequest().source(source); + complexSearchRequest.requestCache(false); + complexSearchRequest.scroll(new TimeValue(1000)); + SearchRequest clonedComplexRequest = complexSearchRequest.clone(); + assertEquals(complexSearchRequest.hashCode(), clonedComplexRequest.hashCode()); + assertNotSame(complexSearchRequest, clonedComplexRequest); } public void testWithLocalReduction() { From 07509068b7597b9cbb6cbec7544650545d68acae Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Tue, 23 Apr 2024 16:28:42 -0700 Subject: [PATCH 3/3] update PR based on comments Signed-off-by: Chenyang Ji --- CHANGELOG.md | 1 + .../action/search/SearchRequest.java | 19 +++++------------ .../action/search/SearchRequestTests.java | 21 +++++++++++++++---- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac0d64558e75..ba55a80f5d1d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote Store] Add settings for remote path type and hash algorithm ([#13225](https://github.com/opensearch-project/OpenSearch/pull/13225)) - [Remote Store] Upload remote paths during remote enabled index creation ([#13386](https://github.com/opensearch-project/OpenSearch/pull/13386)) - [Search Pipeline] Handle default pipeline for multiple indices ([#13276](https://github.com/opensearch-project/OpenSearch/pull/13276)) +- Add support for deep copying SearchRequest ([#12295](https://github.com/opensearch-project/OpenSearch/pull/12295)) ### Dependencies - Bump `org.apache.commons:commons-configuration2` from 2.10.0 to 2.10.1 ([#12896](https://github.com/opensearch-project/OpenSearch/pull/12896)) diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 4a2e463d5418a..4d3bb868b779a 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -167,20 +167,11 @@ public SearchRequest(String[] indices, SearchSourceBuilder source) { * * @return a copy of the current SearchRequest */ - @Override - public SearchRequest clone() { - try (BytesStreamOutput out = new BytesStreamOutput()) { - try { - this.writeTo(out); - } catch (IOException e) { - throw new IllegalArgumentException(e); - } - try (StreamInput in = out.bytes().streamInput()) { - return new SearchRequest(in); - } catch (IOException e) { - throw new IllegalArgumentException(e); - } - } + public SearchRequest deepCopy() throws IOException { + BytesStreamOutput out = new BytesStreamOutput(); + this.writeTo(out); + StreamInput in = out.bytes().streamInput(); + return new SearchRequest(in); } /** diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java index 6468f69cc82e8..40514c526f190 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java @@ -79,18 +79,31 @@ protected SearchRequest createSearchRequest() throws IOException { public void testClone() throws IOException { SearchRequest searchRequest = new SearchRequest(); - SearchRequest clonedRequest = searchRequest.clone(); + SearchRequest clonedRequest = searchRequest.deepCopy(); assertEquals(searchRequest.hashCode(), clonedRequest.hashCode()); assertNotSame(searchRequest, clonedRequest); - SearchSourceBuilder source = new SearchSourceBuilder() - .fetchSource(new FetchSourceContext(true, new String[] { "field1.*" }, new String[] { "field2.*" })); + String[] includes = new String[] { "field1.*" }; + String[] excludes = new String[] { "field2.*" }; + FetchSourceContext fetchSourceContext = new FetchSourceContext(true, includes, excludes); + SearchSourceBuilder source = new SearchSourceBuilder().fetchSource(fetchSourceContext); SearchRequest complexSearchRequest = createSearchRequest().source(source); complexSearchRequest.requestCache(false); complexSearchRequest.scroll(new TimeValue(1000)); - SearchRequest clonedComplexRequest = complexSearchRequest.clone(); + SearchRequest clonedComplexRequest = complexSearchRequest.deepCopy(); assertEquals(complexSearchRequest.hashCode(), clonedComplexRequest.hashCode()); assertNotSame(complexSearchRequest, clonedComplexRequest); + assertEquals(fetchSourceContext, clonedComplexRequest.source().fetchSource()); + assertNotSame(fetchSourceContext, clonedComplexRequest.source().fetchSource()); + // Change the value of the original includes array and excludes array + includes[0] = "new_field1.*"; + excludes[0] = "new_field2.*"; + // Values in the original fetchSource object should be updated + assertEquals("new_field1.*", complexSearchRequest.source().fetchSource().includes()[0]); + assertEquals("new_field2.*", complexSearchRequest.source().fetchSource().excludes()[0]); + // Values in the cloned fetchSource object should not be updated + assertEquals("field1.*", clonedComplexRequest.source().fetchSource().includes()[0]); + assertEquals("field2.*", clonedComplexRequest.source().fetchSource().excludes()[0]); } public void testWithLocalReduction() {