BWC (rag processor): add version control for newly added request para…

…ms (#3125) (#3363) * fix: handle bwc for new fields added in 2.13 Signed-off-by: Pavan Yekbote <[email protected]> * fix: spotless changes Signed-off-by: Pavan Yekbote <[email protected]> * fix: test cases with bwc Signed-off-by: Pavan Yekbote <[email protected]> * fix: spotless apply Signed-off-by: Pavan Yekbote <[email protected]> --------- Signed-off-by: Pavan Yekbote <[email protected]> (cherry picked from commit 40c4e68) Co-authored-by: Pavan Yekbote <[email protected]>
opensearch-project · Jan 9, 2025 · 991090d · 991090d
1 parent ec79f02
commit 991090d
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 10 deletions.
diff --git a/...g/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java b/...g/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java
@@ -22,13 +22,15 @@
 import java.io.IOException;
 import java.util.Objects;
 
+import org.opensearch.Version;
 import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.Writeable;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.core.xcontent.XContentParser;
+import org.opensearch.ml.common.CommonValue;
 
 import com.google.common.base.Preconditions;
 
@@ -81,6 +83,8 @@ public class GenerativeQAParameters implements Writeable, ToXContentObject {
 
     public static final int SIZE_NULL_VALUE = -1;
 
+    public static final Version MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS = CommonValue.VERSION_2_13_0;
+
     @Setter
     @Getter
     private String conversationId;
@@ -145,15 +149,23 @@ public GenerativeQAParameters(
     }
 
     public GenerativeQAParameters(StreamInput input) throws IOException {
+        Version version = input.getVersion();
         this.conversationId = input.readOptionalString();
         this.llmModel = input.readOptionalString();
         this.llmQuestion = input.readString();
-        this.systemPrompt = input.readOptionalString();
-        this.userInstructions = input.readOptionalString();
+
+        if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) {
+            this.systemPrompt = input.readOptionalString();
+            this.userInstructions = input.readOptionalString();
+        }
+
         this.contextSize = input.readInt();
         this.interactionSize = input.readInt();
         this.timeout = input.readInt();
-        this.llmResponseField = input.readOptionalString();
+
+        if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) {
+            this.llmResponseField = input.readOptionalString();
+        }
     }
 
     @Override
@@ -201,17 +213,25 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
+        Version version = out.getVersion();
         out.writeOptionalString(conversationId);
         out.writeOptionalString(llmModel);
 
         Preconditions.checkNotNull(llmQuestion, "llm_question must not be null.");
         out.writeString(llmQuestion);
-        out.writeOptionalString(systemPrompt);
-        out.writeOptionalString(userInstructions);
+
+        if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) {
+            out.writeOptionalString(systemPrompt);
+            out.writeOptionalString(userInstructions);
+        }
+
         out.writeInt(contextSize);
         out.writeInt(interactionSize);
         out.writeInt(timeout);
-        out.writeOptionalString(llmResponseField);
+
+        if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) {
+            out.writeOptionalString(llmResponseField);
+        }
     }
 
     public static GenerativeQAParameters parse(XContentParser parser) throws IOException {

diff --git a/...ch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java b/...ch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java
@@ -21,13 +21,15 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 import static org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS;
 
 import java.io.EOFException;
 import java.io.IOException;
 import java.util.Collections;
 
 import org.junit.Assert;
+import org.opensearch.Version;
 import org.opensearch.common.io.stream.BytesStreamOutput;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentType;
@@ -105,10 +107,17 @@ public void testMiscMethods() throws IOException {
         assertNotEquals(builder1, builder2);
         assertNotEquals(builder1.hashCode(), builder2.hashCode());
 
-        StreamOutput so = mock(StreamOutput.class);
-        builder1.writeTo(so);
-        verify(so, times(5)).writeOptionalString(any());
-        verify(so, times(1)).writeString(any());
+        StreamOutput so1 = mock(StreamOutput.class);
+        when(so1.getVersion()).thenReturn(GenerativeQAParameters.MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS);
+        builder1.writeTo(so1);
+        verify(so1, times(5)).writeOptionalString(any());
+        verify(so1, times(1)).writeString(any());
+
+        StreamOutput so2 = mock(StreamOutput.class);
+        when(so2.getVersion()).thenReturn(Version.V_2_12_0);
+        builder1.writeTo(so2);
+        verify(so2, times(2)).writeOptionalString(any());
+        verify(so1, times(1)).writeString(any());
     }
 
     public void testParse() throws IOException {