update PR based on comments

Signed-off-by: Chenyang Ji <[email protected]>
opensearch-project · Apr 23, 2024 · 4793280 · 4793280
1 parent 1ba2969
commit 4793280
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add an individual setting of rate limiter for segment replication ([#12959](https://github.com/opensearch-project/OpenSearch/pull/12959))
 - [Streaming Indexing] Ensure support of the new transport by security plugin ([#13174](https://github.com/opensearch-project/OpenSearch/pull/13174))
 - Add cluster setting to dynamically configure the buckets for filter rewrite optimization. ([#13179](https://github.com/opensearch-project/OpenSearch/pull/13179))
+- 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
@@ -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
@@ -79,18 +79,32 @@ 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);
 
+        String[] includes = new String[] { "field1.*" };
+        String[] excludes = new String[] { "field2.*" };
+        FetchSourceContext fetchSourceContext = new FetchSourceContext(true, includes, excludes);
         SearchSourceBuilder source = new SearchSourceBuilder()
-            .fetchSource(new FetchSourceContext(true, new String[] { "field1.*" }, new String[] { "field2.*" }));
+            .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() {